-
-
Notifications
You must be signed in to change notification settings - Fork 4
INFRA-3187: Automate Merging Release Branch workflow #186
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
Signed-off-by: Pavel Dvorkin <pavel.dvorkin@consensys.net>
29c17fe to
ed5be17
Compare
| - name: Checkout GitHub tools repository | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| repository: metamask/github-tools | ||
| path: ./github-tools |
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.
This will check out the main branch of github-tools, which is prone to breaking. Please change this workflow to an action instead, and use this pattern:
github-tools/.github/actions/check-changelog/action.yml
Lines 20 to 27 in 1036dfd
| github-tools-repository: | |
| description: 'The GitHub repository containing the GitHub tools. Defaults to the GitHub tools action repositor, and usually does not need to be changed.' | |
| required: false | |
| default: ${{ github.action_repository }} | |
| github-tools-ref: | |
| description: 'The SHA of the action to use. Defaults to the current action ref, and usually does not need to be changed.' | |
| required: false | |
| default: ${{ github.action_ref }} |
github-tools/.github/actions/check-changelog/action.yml
Lines 65 to 71 in 1036dfd
| - name: Checkout GitHub tools repository | |
| if: ${{ steps.label-check.outputs.skip_check != 'true' }} | |
| uses: actions/checkout@v6 | |
| with: | |
| repository: ${{ inputs.github-tools-repository }} | |
| ref: ${{ inputs.github-tools-ref }} | |
| path: ./.github-tools |
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.
Done
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 this workflow necessary now? The action can be called from other repositories directly.
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.
Good call. I removed the workflow
| token: ${{ secrets.github-token || secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Merge previous releases | ||
| uses: metamask/github-tools/.github/actions/merge-previous-releases@INFRA-3187-AutomateMergingReleaseBranches |
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.
Bug: Workflow references feature branch that will be deleted
The workflow references the action using @INFRA-3187-AutomateMergingReleaseBranches, which is a feature branch name. Once this PR is merged, this branch will likely be deleted, causing the workflow to fail when it tries to fetch the action. This aligns with the reviewer comment from @Mrtenz who noted that referencing the main branch is "prone to breaking" and suggested using a proper versioning pattern with github.action_repository and github.action_ref.
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.
This was in the workflow and has been removed
Mrtenz
left a comment
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.
Action looks good to me. I don't have context on the script itself, so will let someone else review that!
| git config --global user.name "github-actions[bot]" | ||
| git config --global user.email "github-actions[bot]@users.noreply.github.com" |
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.
Do we want this to be metamaskbot?
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.
You're right, these are just used for commit metadata, but let me update it to metamaskbot like our other workflows. Fixed
| if [[ -n "$file" ]]; then | ||
| echo " - Conflict in: ${file} → keeping destination version" | ||
| git_exec checkout --ours "$file" | ||
| git_exec add "$file" |
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.
Bug: Delete/modify conflicts may cause script failure
When resolving merge conflicts, git checkout --ours "$file" will fail if the file was deleted in the destination branch ("ours") but modified in the source branch. Git reports "error: path does not have our version" and exits with non-zero status. With set -e enabled, this causes the entire script to abort instead of gracefully handling the conflict by keeping the deletion. This affects modify/delete conflicts that aren't resolved by the initial git merge -X ours command.
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.
Fixed
Note
Adds a composite GitHub Action and Bash script to automatically merge all older
release/*branches into a newly created release branch, favoring the new branch on conflicts and pushing if merges occur./.github/actions/merge-previous-releases/action.ymlto merge previousrelease/*branches into a new release branch./.github/scripts/merge-previous-releases.sh:release/X.Y.Z, enumerates older release branches, sorts them, and merges into the new branch.-X oursto favor destination on conflicts and resolves remaining conflicts programmatically.github-toolsfrom staging, and pushes only if any merges occurred.Written by Cursor Bugbot for commit 535b887. This will update automatically on new commits. Configure here.
Testing:
consensys-test/metamask-mobile-test-workflow#59