-
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
base: main
Are you sure you want to change the base?
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...
contents: write # needed to push the updated files | ||
pull-requests: write # needed to create the PR | ||
env: | ||
RELEASE_TAG: ${{ inputs.tag || 'codeql-bundle-v2.22.0' }} |
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.
What is the plan for this default value moving forward?
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.
That's part of the test commit, which I noted in the PR description will be removed. So there will be no default value, and the tag
input is required for the workflow_dispatch
event.
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. |
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