INFRA-3041: Stable sync workflow fixes#154
Conversation
Signed-off-by: Pavel Dvorkin <pavel.dvorkin@consensys.net>
gauthierpetetin
left a comment
There was a problem hiding this comment.
Thanks for the PR, I added a few comments.
|
|
||
| // Preserve package.json from stable branch (no version bump) | ||
| await exec(`git checkout origin/${baseBranch} -- package.json`); | ||
| console.log(`Executed: git checkout origin/${baseBranch} -- package.json`); |
There was a problem hiding this comment.
We don't need to preserve package.json from stable branch. We can delete this line which will leave us without extension-specific commands.
For reference, we want to reproduce the logic that's currently in stable-sync-legacy.js script, i.e. create a stable-sync branch that will be merged into main while:
- Bringing commit history from
stable(the cherry-picked commits) intomain - Bringing CHANGELOG.md updates from
stableintomain - Avoiding code conflicts because the code changes are already in
main(they were cherry-picked to create the release)
There was a problem hiding this comment.
Ok, I was under the impression we wanted to preserve it. Will remove the extra logic here. Thanks
| git config submodule.recurse false | ||
| git config diff.ignoreSubmodules all | ||
|
|
||
| # Start the rebase |
There was a problem hiding this comment.
I wonder whether we really need a rebase. Can we not just override the origin/stable-main-${{ inputs.semver-version }} branch with a git push force?
| # Check if branch exists remotely | ||
| echo "Cleaning up github-tools" | ||
| rm -rf github-tools | ||
| # Ensure github-tools is not staged or tracked |
There was a problem hiding this comment.
I wonder where the github-tools file comes from. Maybe instead of introducing all this logic to avoid including it in the commit, we could just add it to .gitignore?
| echo "github-tools/" >> .gitignore | ||
| echo "Added github-tools/ to .gitignore" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Bug: Gitignore Overwritten by Git Commands Reinstating Exclusions
The workflow adds github-tools/ to .gitignore to prevent its commitment. However, the stable-sync.js script's git commands restore files from origin/main, overwriting .gitignore and undoing the exclusion. This means github-tools/ may still be committed.
There was a problem hiding this comment.
No I don't think it's relevant. We're adding github-tools/ to .gitignore in mobile, extension also so it shouldn't be an issue once we do that.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Ticket: https://consensyssoftware.atlassian.net/browse/INFRA-3041 Github-tools PR: MetaMask/github-tools#154 Summary of Changes Prevented github-tools from appearing in PRs File: github-tools/.github/workflows/stable-sync.yml What: Added explicit cleanup commands to unstage and remove github-tools directory before pushing Impact: The sync PR will no longer include a github-tools/ submodule or directory Disabled package.json version bump for Extension File: github-tools/.github/scripts/stable-sync.js What: Removed the yarn version logic that bumped package.json version Before: Extension ran yarn version to update the version After: Extension now preserves package.json from stable branch (same as mobile) Impact: No version bump will occur in the sync PR for either mobile or extension Verified PR title format File: github-tools/.github/workflows/stable-sync.yml (line 147) What: Confirmed it already uses release: prefix (no change needed) Format: "release: sync stable to main for version X.Y.Z" Testing in Extension: consensys-test#209, https://github.com/consensys-test/metamask-extension-test-workflow2/actions/runs/18951817173 Testing in Mobile: consensys-test/metamask-mobile-test-workflow#30, https://github.com/consensys-test/metamask-mobile-test-workflow/actions/runs/18951842114 Testing Post CR comments: consensys-test#212 ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: None ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Bumps stable-branch sync workflow to a new github-tools ref and ignores the `github-tools/` directory. > > - **CI/Workflows**: > - Update `.github/workflows/stable-branch-sync.yml` to use `metamask/github-tools/.github/workflows/stable-sync.yml@79ce6e3` and set `github-tools-version` to `79ce6e3`. > - **Repo hygiene**: > - Add `github-tools/` to `.gitignore`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f8418e5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Ticket: https://consensyssoftware.atlassian.net/browse/INFRA-3041 Github-tools PR: MetaMask/github-tools#154, MetaMask/github-tools#160 Changelog PR before updates: consensys-test#31 Changelog PR after updates: consensys-test#37 Summary of Changes Prevented github-tools from appearing in PRs File: github-tools/.github/workflows/stable-sync.yml What: Added explicit cleanup commands to unstage and remove github-tools directory before pushing Impact: The sync PR will no longer include a github-tools/ submodule or directory Disabled package.json version bump for Extension File: github-tools/.github/scripts/stable-sync.js What: Removed the yarn version logic that bumped package.json version Before: Extension ran yarn version to update the version After: Extension now preserves package.json from stable branch (same as mobile) Impact: No version bump will occur in the sync PR for either mobile or extension Verified PR title format File: github-tools/.github/workflows/stable-sync.yml (line 147) What: Confirmed it already uses release: prefix (no change needed) Format: "release: sync stable to main for version X.Y.Z" Testing in Extension: consensys-test/metamask-extension-test-workflow2#209, https://github.com/consensys-test/metamask-extension-test-workflow2/actions/runs/18951817173 Testing in Mobile: consensys-test#30, https://github.com/consensys-test/metamask-mobile-test-workflow/actions/runs/18951842114 Testing Post CR comments: consensys-test/metamask-extension-test-workflow2#212 ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: None ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Updates the stable branch sync workflow to use metamask/github-tools commit 701a894 for both the reusable workflow and the input version. > > - **CI/Workflows**: > - Update `uses` in `.github/workflows/stable-branch-sync.yml` to `metamask/github-tools/.github/workflows/stable-sync.yml@701a894f38883ab48560f948e98b76cc6b4d623f`. > - Set `github-tools-version` input to `701a894f38883ab48560f948e98b76cc6b4d623f`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 77f5118. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Ticket: https://consensyssoftware.atlassian.net/browse/INFRA-3041
Summary of Changes
File: github-tools/.github/workflows/stable-sync.yml
What: Added explicit cleanup commands to unstage and remove github-tools directory before pushing
Impact: The sync PR will no longer include a github-tools/ submodule or directory
File: github-tools/.github/scripts/stable-sync.js
What: Removed the yarn version logic that bumped package.json version
Before: Extension ran yarn version to update the version
After: Extension now preserves package.json from stable branch (same as mobile)
Impact: No version bump will occur in the sync PR for either mobile or extension
File: github-tools/.github/workflows/stable-sync.yml (line 147)
What: Confirmed it already uses release: prefix (no change needed)
Format: "release: sync stable to main for version X.Y.Z"
Testing in Extension: consensys-test/metamask-extension-test-workflow2#209, https://github.com/consensys-test/metamask-extension-test-workflow2/actions/runs/18951817173
Testing in Mobile: consensys-test/metamask-mobile-test-workflow#30, https://github.com/consensys-test/metamask-mobile-test-workflow/actions/runs/18951842114
Testing Post CR comments: consensys-test/metamask-extension-test-workflow2#212
Note
Stabilizes the sync process by ignoring github-tools in PRs, removing the extension version bump, and force-pushing existing stable sync branches.
Workflows:
stable-sync.yml:github-tools/to.gitignoreto prevent committing it.stable-main-<version>branch exists; set upstream on first push.Scripts:
github-tools/.github/scripts/stable-sync.js:package.jsonversion bump; only run mobile-specific file checkouts.Written by Cursor Bugbot for commit ee74d68. This will update automatically on new commits. Configure here.