-
Notifications
You must be signed in to change notification settings - Fork 363
Add workflow for updating release used by start-proxy
#2941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7c89bca
to
e8ad3af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new GitHub Actions workflow to automate updating the start-proxy
action’s release assets and opening draft PRs.
- Adds a workflow dispatch and temporary push trigger for testing
- Defines steps to replace URLs and versions in code, build, and open a PR
- Configures branch naming and GitHub permissions for the bot
Comments suppressed due to low confidence (3)
.github/workflows/update-proxy-release.yml:5
- Remove the temporary push trigger on the
mbg/update-proxy-binaries
branch before merging, as this is only needed for testing.
- mbg/update-proxy-binaries # for testing
.github/workflows/update-proxy-release.yml:55
- [nitpick] Trim the leading spaces in the heredoc for
pr_body
so the PR description renders as normal text instead of a code block.
This PR updates the \`start-proxy\` action to use the private registry proxy binaries that
.github/workflows/update-proxy-release.yml:44
- [nitpick] Avoid hard-coding the
v2.0
prefix; derive the full release version from theRELEASE_TAG
input so that future major/minor bumps don’t require patching this script.
sed -i '' "s/\"v2.0.[0-9]*\"/\"v2.0.$NOW\"/g" ./src/start-proxy-action.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there prior art for doing releases like this?
I'm not a huge fan of doing text replacements in typescript files with workflow inputs...
Agreed, perhaps a check that the tag is in the expected format before proceeding would be good?
Depends on what you are asking about specifically. This PR just automates a few steps we'd have to perform manually otherwise. The whole arrangement with how the |
Yes. Let's add that in. It's also nice to have to prevent silly bugs.
NP. Thanks for the extra context. |
500d1d1
to
5446556
Compare
45c20ef
to
6e22e41
Compare
Done, along with an additional check to make sure that the release actually exists. We could take this further to check that the release assets include the three expected files, but this is probably enough sanity checking in the workflow for now -- I am not expecting this to be used much. I'd only expect an increase in usage if we automate the process end-to-end, but we may well choose to store the proxy binaries elsewhere then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great.
Except for the remaining testing knobs this is in a fine shape.
I also did a general review of the Actions parts of this and have a a few isolated comments.
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
with: | ||
fetch-depth: 0 # ensure we have all tags and can push commits | ||
ref: main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we move this checkout earlier than the Check that the release exists
step, then gh
will pick up the repository to query automatically, allowing us to drop the --repo
. Checkout is fast enough that it can go before the error checking IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had considered that, but explicitly adding the --repo
argument isn't much of an inconvenience so it makes more sense to me to perform the check first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
First workflow run on Resulting PR: #2947 Looks like it is working fine outside of test conditions, too. |
This PR adds a new workflow which automates the creation of PRs that update which release is used by the
start-proxy
action to pull binaries of theupdate-job-proxy
from.Previously, I manually created PRs such as this one for that. The new workflow here creates similar ones, such as this example.
This PR currently contains a commit that adds a
push
trigger (limited to this branch) for testing, which should be removed before merging this PR.Merge / deployment checklist