-
-
Notifications
You must be signed in to change notification settings - Fork 4
Fix: Prevent changelog PR creation when branches are in sync #177
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
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 fixes an issue where the changelog update script would fail when attempting to create a PR for branches that are already in sync (have no differences). This scenario occurs after a changelog PR has been merged, when the script runs again and finds no new changes to commit.
Key Changes:
- Tracks whether changelog changes were successfully committed using a boolean flag
- When no commit occurs, compares the changelog branch against the release branch to detect any differences
- Returns successfully and skips PR creation if branches are in sync, preventing the "No commits between branches" GraphQL error
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
5fbc261 to
336efef
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if there are any differences between the changelog branch and release branch | ||
| if ! ${changes_committed}; then | ||
| echo "Checking for differences between ${changelog_branch} and ${release_branch}.." | ||
| if ! git diff --quiet "origin/${release_branch}" "HEAD"; then | ||
| echo "Differences found between branches; proceeding with PR creation." | ||
| else | ||
| echo "No differences between ${changelog_branch} and ${release_branch}." | ||
| echo "Branches are already in sync; skipping PR creation." | ||
| echo "Changelog workflow completed successfully (no updates needed)." | ||
| return 0 | ||
| fi | ||
| fi |
Copilot
AI
Dec 4, 2025
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.
The new branch synchronization check logic (lines 177-188) lacks test coverage. Since the repository has test scripts for similar shell scripts (e.g., test-create-platform-release-pr-full.sh and test-create-platform-release-pr-functions.sh), consider adding a test file like .github/scripts/tests/test-update-release-changelog.sh that validates:
- The script exits gracefully when branches are in sync (no diff)
- The script proceeds with PR creation when differences exist
- The
changes_committedflag is properly tracked
| echo "No changes detected; skipping commit." | ||
| fi | ||
|
|
||
| # Check if there are any differences between the changelog branch and release branch |
Copilot
AI
Dec 4, 2025
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.
[nitpick] The comment could be more descriptive about the purpose of this check. Consider updating it to:
# If no changes were committed, check if the branches are already in sync
# This handles the post-merge scenario where the changelog PR was already merged
This makes it clearer why we're performing this check only when changes_committed is false.
| # Check if there are any differences between the changelog branch and release branch | |
| # If no changes were committed, check if the branches are already in sync | |
| # This handles the post-merge scenario where the changelog PR was already merged |
## **Description** Updates `github-tools` to v1.1.3 across release workflows. ### Changes - Updated `create-release-pr.yml` to use `github-tools@v1.1.3` - Updated `update-release-changelog.yml` to use `github-tools@v1.1.3` ### What's included in v1.1.3 - **Fixed**: Prevent changelog PR creation when branches are in sync ([#177](MetaMask/github-tools#177)) See [github-tools v1.1.3 release notes](https://github.com/MetaMask/github-tools/releases/tag/v1.1.3). [](https://codespaces.new/MetaMask/metamask-extension/pull/38588?quickstart=1) ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/INFRA-3167 ## **Manual testing steps** 1. This is a workflow configuration change - no manual testing required 2. Changes will be validated during the next release cycle ## **Screenshots/Recordings** N/A - workflow configuration change only ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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.
## **Description** Updates `github-tools` to v1.1.3 in release workflows. ### Changes - Updated `update-release-changelog.yml` to use `github-tools@v1.1.3` ### What's included in v1.1.3 - **Fixed**: Prevent changelog PR creation when branches are in sync ([#177](MetaMask/github-tools#177)) See [github-tools v1.1.3 release notes](https://github.com/MetaMask/github-tools/releases/tag/v1.1.3). ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/INFRA-3167 ## **Manual testing steps** ```gherkin Feature: Release changelog workflow Scenario: Workflow uses updated github-tools version Given the workflow configuration is updated When a release branch receives a push Then the workflow uses github-tools v1.1.3 ``` ## **Screenshots/Recordings** N/A - workflow configuration change only ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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.
## **Description** Updates `github-tools` to v1.1.3 in release workflows. ### Changes - Updated `update-release-changelog.yml` to use `github-tools@v1.1.3` ### What's included in v1.1.3 - **Fixed**: Prevent changelog PR creation when branches are in sync ([#177](MetaMask/github-tools#177)) See [github-tools v1.1.3 release notes](https://github.com/MetaMask/github-tools/releases/tag/v1.1.3). ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/INFRA-3167 ## **Manual testing steps** ```gherkin Feature: Release changelog workflow Scenario: Workflow uses updated github-tools version Given the workflow configuration is updated When a release branch receives a push Then the workflow uses github-tools v1.1.3 ``` ## **Screenshots/Recordings** N/A - workflow configuration change only ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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.
What is the current state of things and why does it need to change?
When a changelog PR is merged and then a new commit triggers the workflow again, the workflow attempts to create a new PR even when there are no changes between the changelog branch and the release branch. This results in the GraphQL "No commits between branches" error.
What is the solution your changes offer and how does it work?
The fix adds logic to gracefully handle the post-merge scenario:
changes_committedboolean)Result: The script now handles the post-merge scenario where both branches are identical without failing.
Testing
Tested in consensys-test/metamask-extension-test using the
fix/INFRA-3167-manage-no-diffbranch:Test Scenario: "No Diff" - Branches in Sync After Merge
release/6000.0.0branchchanges_committed=falseFinal State:
37624ba7c9(merge commit only)7b37e1923d(merged PR)Related Issues