-
-
Notifications
You must be signed in to change notification settings - Fork 274
feat: add Update Changelogs workflow with auto-changelog v6 --checkDeps #8443
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?
Changes from all commits
61175c2
a83983a
02fd302
14d0298
9c0924f
f3192c6
925ca11
a62d2ee
b6cf542
b516c86
455620a
9069a65
5a137b1
61a4b9e
f4aabbb
e8bd7be
30eec0c
b20f53e
29a5a04
d29dc62
567a53b
0959ecc
bbac608
aa78ed8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||||||
| name: Update Changelogs | ||||||||||||
|
|
||||||||||||
| on: | ||||||||||||
| issue_comment: | ||||||||||||
| types: [created] | ||||||||||||
| pull_request: | ||||||||||||
| branches: [main] | ||||||||||||
| types: [opened] | ||||||||||||
|
|
||||||||||||
| concurrency: | ||||||||||||
| group: update-changelogs-${{ github.event.issue.number || github.event.pull_request.number }} | ||||||||||||
| cancel-in-progress: true | ||||||||||||
|
|
||||||||||||
| permissions: | ||||||||||||
| contents: write | ||||||||||||
| pull-requests: write | ||||||||||||
|
|
||||||||||||
| jobs: | ||||||||||||
| is-fork-pull-request: | ||||||||||||
| name: Determine whether this PR is from a fork | ||||||||||||
| if: > | ||||||||||||
| (github.event_name == 'pull_request' && startsWith(github.head_ref, 'release/')) || | ||||||||||||
| (github.event.issue.pull_request && contains(github.event.comment.body, '@metamaskbot update-changelogs')) | ||||||||||||
|
cryptodev-2s marked this conversation as resolved.
|
||||||||||||
| runs-on: ubuntu-latest | ||||||||||||
| outputs: | ||||||||||||
| IS_FORK: ${{ steps.is-fork.outputs.IS_FORK }} | ||||||||||||
| steps: | ||||||||||||
| - name: Determine whether this PR is from a fork | ||||||||||||
| id: is-fork | ||||||||||||
| run: | | ||||||||||||
| IS_FORK=$(gh pr view --json isCrossRepository --jq '.isCrossRepository' "$PR_NUMBER" --repo "$GITHUB_REPOSITORY") | ||||||||||||
| echo "IS_FORK=$IS_FORK" >> "$GITHUB_OUTPUT" | ||||||||||||
| env: | ||||||||||||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||
| PR_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} | ||||||||||||
|
|
||||||||||||
| update-changelogs: | ||||||||||||
| name: Update changelogs | ||||||||||||
| needs: is-fork-pull-request | ||||||||||||
| if: ${{ needs.is-fork-pull-request.outputs.IS_FORK == 'false' }} | ||||||||||||
| runs-on: ubuntu-latest | ||||||||||||
| timeout-minutes: 30 | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought jobs automatically timed out? I see that you borrowed this from the extension repo, but we haven't been specifying timeouts in this repo so I'm curious if it's really necessary.
Suggested change
|
||||||||||||
| env: | ||||||||||||
| GITHUB_TOKEN: ${{ secrets.UPDATE_CHANGELOG_TOKEN }} | ||||||||||||
| PR_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }} | ||||||||||||
| steps: | ||||||||||||
| - name: React to comment | ||||||||||||
| if: github.event_name == 'issue_comment' | ||||||||||||
| continue-on-error: true | ||||||||||||
| run: gh api "repos/${GITHUB_REPOSITORY}/issues/comments/${COMMENT_ID}/reactions" -f content='+1' | ||||||||||||
| env: | ||||||||||||
| GH_TOKEN: ${{ github.token }} | ||||||||||||
| COMMENT_ID: ${{ github.event.comment.id }} | ||||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||||
|
|
||||||||||||
| - name: Checkout repository | ||||||||||||
| uses: actions/checkout@v4 | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use v5:
Suggested change
|
||||||||||||
| with: | ||||||||||||
| # Use PAT to ensure the push triggers subsequent CI workflows | ||||||||||||
| token: ${{ secrets.UPDATE_CHANGELOG_TOKEN }} | ||||||||||||
| fetch-depth: 0 | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it necessary to pull all history? We don't do this for the
Suggested change
Besides, in the "Setup environment" step below we already check out the repo with |
||||||||||||
|
|
||||||||||||
| - name: Checkout pull request | ||||||||||||
| run: gh pr checkout "$PR_NUMBER" | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not really sure why we have to check out the PR specifically, but I guess we do this elsewhere, so there must be a reason 🤔 |
||||||||||||
|
|
||||||||||||
| - name: Setup environment | ||||||||||||
| uses: MetaMask/action-checkout-and-setup@v2 | ||||||||||||
| with: | ||||||||||||
| is-high-risk-environment: false | ||||||||||||
| cache-node-modules: true | ||||||||||||
| node-version: 22.x | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't specify the Node version if we don't have to. If we ever upgrade Node we would want this to use the latest version. I believe we can leave this off and that way
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setup action re-checkouts, overwriting PR branchHigh Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit aa78ed8. Configure here. |
||||||||||||
|
|
||||||||||||
| - name: Hide previous bot comments | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should only do this once we've confirmed that we need to post a new comment? |
||||||||||||
| continue-on-error: true | ||||||||||||
| run: | | ||||||||||||
| COMMENT_IDS=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" --paginate \ | ||||||||||||
| --jq '[.[] | select(.user.login == "github-actions[bot]" and (.body | test("^(✅|⚠️|❌)"))) | .node_id] | .[]') | ||||||||||||
| for NODE_ID in $COMMENT_IDS; do | ||||||||||||
| gh api graphql -f query='mutation { minimizeComment(input: {subjectId: "'"$NODE_ID"'", classifier: OUTDATED}) { clientMutationId } }' | ||||||||||||
| done | ||||||||||||
| env: | ||||||||||||
| GH_TOKEN: ${{ github.token }} | ||||||||||||
|
|
||||||||||||
| - name: Validate and fix dependency bump entries | ||||||||||||
| id: validate | ||||||||||||
|
Comment on lines
+83
to
+84
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we use a more descriptive name?
Suggested change
|
||||||||||||
| run: > | ||||||||||||
| yarn workspaces foreach --all --no-private --parallel --interlaced --verbose | ||||||||||||
| run changelog:validate --checkDeps --fix --currentPr "$PR_NUMBER" | ||||||||||||
|
Comment on lines
+85
to
+87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we reuse the package script in the root?
Suggested change
|
||||||||||||
| continue-on-error: true | ||||||||||||
|
|
||||||||||||
| - name: Commit and push if changed | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we use a more descriptive name?
Suggested change
|
||||||||||||
| id: commit | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we use a more descriptive step ID?
Suggested change
|
||||||||||||
| run: | | ||||||||||||
| if git diff --quiet; then | ||||||||||||
|
mcmire marked this conversation as resolved.
|
||||||||||||
| echo "changed=false" >> "$GITHUB_OUTPUT" | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name of this output seems to indicate that tracks whether there were changes, but this is inaccurate. This tracks not only whether there were changes, but also whether they were pushed. Maybe this should be:
Suggested change
|
||||||||||||
| exit 0 | ||||||||||||
| fi | ||||||||||||
| git diff --stat | ||||||||||||
| git config user.name "github-actions[bot]" | ||||||||||||
| git config user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||||||||||||
|
Comment on lines
+98
to
+99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessary? I thought using the token to check out the repo automatically sets the user name and email.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes — |
||||||||||||
| git add -- '**/CHANGELOG.md' | ||||||||||||
| git commit -m "chore: auto-fix dependency bump changelog entries" | ||||||||||||
| git push | ||||||||||||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
|
||||||||||||
| - name: Comment result | ||||||||||||
| if: always() | ||||||||||||
| run: | | ||||||||||||
| if [ "$CHANGED" = "true" ] && [ "$VALIDATE_OUTCOME" = "failure" ]; then | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "⚠️ Changelogs updated and pushed, but some validation errors remain. Check the [workflow run]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) for details." | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder how this could happen 🤔 This would seem to indicate that changes were pushed when they were not supposed to be. |
||||||||||||
| elif [ "$CHANGED" = "true" ]; then | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "✅ Changelogs updated and pushed." | ||||||||||||
| elif [ "$COMMIT_OUTCOME" = "failure" ]; then | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "❌ Failed to push changelog fixes. Check the [workflow run]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) for details." | ||||||||||||
| elif [ "$VALIDATE_OUTCOME" = "failure" ]; then | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "❌ Changelog validation failed. Check the [workflow run]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) for details." | ||||||||||||
| elif [ "$VALIDATE_OUTCOME" = "skipped" ] || [ "$COMMIT_OUTCOME" = "skipped" ]; then | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "❌ Workflow failed before changelog validation. Check the [workflow run]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) for details." | ||||||||||||
| else | ||||||||||||
| gh pr comment "$PR_NUMBER" --body "✅ No changelog changes needed." | ||||||||||||
| fi | ||||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||||
| env: | ||||||||||||
| GH_TOKEN: ${{ github.token }} | ||||||||||||
| CHANGED: ${{ steps.commit.outputs.changed }} | ||||||||||||
| COMMIT_OUTCOME: ${{ steps.commit.outcome }} | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we use a more descriptive name for this step?
Suggested change
|
||||||||||||
| VALIDATE_OUTCOME: ${{ steps.validate.outcome }} | ||||||||||||
|
cursor[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we use a more descriptive name for this step?
Suggested change
|
||||||||||||


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.
Release branches aren't required to start with
release/. We used to check for release branches this way, but now we have a more sophisticated check. You should be able to use theis-releaseaction to determine this. See how we do this inmain.yml:core/.github/workflows/main.yml
Lines 94 to 105 in 09d5bda
(Maybe we want to extract this step to a separate internal action so we don't have to repeat the list of commit patterns?)