-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
build: add create release proposal action #55690
build: add create release proposal action #55690
Conversation
Review requested:
|
tools/actions/create-release.sh
Outdated
# We use it to not specify the branch name as it changes based on | ||
# the commit list (semver-minor/semver-patch) | ||
git config push.default current | ||
git push upstream |
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.
We probably should not expect to have push permission, and instead make an API call to create the release commit using a createCommitOnBranch
API call
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.
Actually, the gh pr create
also pushes the branch so it should just works? cli/cli#6958
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 I'm saying is: the workflow should not have push permissions, this git push
call is problematic
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.
Why? Wouldn't it fall in the same circumstances as commit-queue.yml
?
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.
I think we don't need that permission, so for that reason, we should not use it, it's good practice to use the smallest set of permissions as possible.
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's your suggestion then? Sorry, I don't get it yet
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.
My suggestion is to use an createCommitOnBranch
API call. I'll try building an alternate proposal using it.
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 do you think about a follow-up PR? I mean, the end goal is to create a proposal using automation, the sooner we land that the sooner we can test it and see how it behaves in practice.
I wish I could use it for next v23 release.
8fa22ec
to
d2b8624
Compare
Uh, just found the action is broken. I will fix it https://github.com/nodejs/node/actions/runs/11804669470 |
452c3c2
to
b055972
Compare
b055972
to
90434fb
Compare
I've been using a fork to test this action and I got some errors related to the changelog token (https://github.com/RafaelGSS/node/actions/runs/11901774519/job/33165408792) However, the action seems to work fine when using a correct token (on my local): ➜ node (v23.x-staging) ./tools/actions/create-release.sh 2024-11-20 23
+ RELEASE_DATE=2024-11-20
+ RELEASE_LINE=23
+ '[' -z 2024-11-20 ']'
+ '[' -z 23 ']'
+ git config --local user.email [email protected]
+ git config --local user.name 'Node.js GitHub Bot'
+ git node release --prepare --skipBranchDiff --yes --releaseDate 2024-11-20
✔ No git cherry-pick in progress
✔ No git am in progress
✔ No git rebase in progress
--------------------------------------------------------------------------------
⠋ Bringing upstream/v23.x-staging up to date...From github.com:nodejs/node
* branch v23.x-staging -> FETCH_HEAD
✔ upstream/v23.x-staging is now up-to-date
v23.x-staging is out of sync with upstream/v23.x-staging. Mismatched commits:
- 94b329eb9b build: add create release proposal action
- def0419a13 fixup! build: add create release proposal action
- d77dfa0868 fixup! fixup! build: add create release proposal action
- 31718928b1 fixup! fixup! fixup! build: add create release proposal action
- df3289b4f7 fixup! fixup! fixup! fixup! build: add create release proposal action
- b406328711 fixup! fixup! fixup! fixup! fixup! build: add create release proposal action
- fee2f6d86a fixup! fixup! fixup! fixup! fixup! fixup! build: add create release proposal action
- 4800a3f098 fixup! fixup! fixup! fixup! fixup! fixup! fixup! build: add create release proposal action
- 5d33422dff fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! build: add create release proposal action
--------------------------------------------------------------------------------
HEAD is now at 693fda0802 lib: remove unused file `fetch_module`
✔ Reset to upstream/v23.x-staging
✔ Latest release on 23.x line is 23.2.0
--------------------------------------------------------------------------------
⠙ Creating new proposal branch for 23.3.0Switched to a new branch 'v23.3.0-proposal'
✔ Created new proposal branch for 23.3.0
✔ Updated 'src/node_version.h' for 23.3.0
✔ Updated REPLACEME items in docs
--------------------------------------------------------------------------------
✔ Updated CHANGELOG.md
⠹ Updating CHANGELOG_V23.mdFrom github.com:nodejs/node
* branch v23.x -> FETCH_HEAD
✔ Updated CHANGELOG_V23.md
--------------------------------------------------------------------------------
From github.com:nodejs/node
* branch v23.x -> FETCH_HEAD
2024-11-20, Version 23.3.0 (Current)
Notable changes:
doc:
* enforce strict policy to semver-major releases (Rafael Gonzaga) https://github.com/nodejs/node/pull/55732
src:
* (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) https://github.com/nodejs/node/pull/55697
util:
* (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) https://github.com/nodejs/node/pull/55589
* (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) https://github.com/nodejs/node/pull/55626
PR-URL: TODO
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
✔ Release preparation for 23.3.0 complete.
ℹ To finish the release proposal, run:
$ git push -u upstream v23.3.0-proposal
Finally, proceed to Jenkins and begin the following CI jobs:
* https://ci.nodejs.org/job/node-test-pull-request/
* https://ci.nodejs.org/job/citgm-smoker/
ℹ If this release has deps/v8 changes, you'll also need to run:
* https://ci.nodejs.org/job/node-test-commit-v8-linux/
+ git config push.default current
++ awk '/^## 2024-11-20/ { print substr($0, 4) }' doc/changelogs/CHANGELOG_V23.md
+ TITLE='2024-11-20, Version 23.3.0 (Current), @RafaelGSS'
++ mktemp
+ TEMP_BODY=/tmp/tmp.aE5l3vXtyn
+ awk '/## 2024-11-20/,/^<a id=/{ if (!/^<a id=/) print }' doc/changelogs/CHANGELOG_V23.md
++ git branch --show-current
+ gh pr create --title '2024-11-20, Version 23.3.0 (Current), @RafaelGSS' --body-file /tmp/tmp.aE5l3vXtyn --head v23.3.0-proposal
Creating pull request for v23.3.0-proposal into main in nodejs/node
pull request create failed: GraphQL: Head sha can't be blank, Base sha can't be blank, No commits between main and v23.3.0-proposal, Head ref must be a branch (createPullRequest) If you note there's an error in the end:
That happens because the v23.0.0-proposal branch wasn't pushed to upstream before the We should push the v23.3.0-proposal to upstream before Result: #55921 |
To summarize:
|
So, IIUC with this approach we would run the automation to create the initial draft and then checkout the PR locally to work on the changelog / notable changes ? |
Not necessarily. This automation will generate the changelog and notable changes automatically (e.g #55921). However, if some notable changes requires a detailed text and then yes, we would need to checkout the PR and adjust the changelog. But, this can also be automated in the future since we have established a pattern of a notable-change label with a summary in the PR description. |
@rvagg Do you think it's reasonable to make changelog-maker to read from
meanwhile |
beafb6b
to
5aa3d35
Compare
Oh, I could test it properly today. Run: https://github.com/RafaelGSS/node/actions/runs/11978160284/job/33397598137 Result RafaelGSS#10 It seems to be working well. PTAL @nodejs/releasers |
# Needs the whole git history for ncu to work | ||
# See https://github.com/nodejs/node-core-utils/pull/486 | ||
fetch-depth: 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.
Is that true for preparing a release?
# Needs the whole git history for ncu to work | |
# See https://github.com/nodejs/node-core-utils/pull/486 | |
fetch-depth: 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.
We can change it later if needed
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.
Could you test on your local clone or on node-auto-test?
87fb909
to
51f5803
Compare
Just tested your suggestions and it also works @aduh95. Thanks. |
Co-authored-by: Antoine du Hamel <[email protected]>
Commit Queue failed- Loading data for nodejs/node/pull/55690 ✔ Done loading data for nodejs/node/pull/55690 ----------------------------------- PR info ------------------------------------ Title build: add create release proposal action (#55690) Author Rafael Gonzaga <[email protected]> (@RafaelGSS) Branch RafaelGSS:add-release-proposal -> nodejs:main Labels meta, tools, author ready Commits 5 - build: add create release proposal action - fixup! build: add create release proposal action - fixup! fixup! build: add create release proposal action - fixup! fixup! fixup! build: add create release proposal action - Update .github/workflows/create-release-proposal.yml Committers 2 - RafaelGSS <[email protected]> - GitHub <[email protected]> PR-URL: https://github.com/nodejs/node/pull/55690 Refs: https://github.com/nodejs/security-wg/issues/860 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/55690 Refs: https://github.com/nodejs/security-wg/issues/860 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 02 Nov 2024 04:02:42 GMT ✔ Approvals: 2 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/55690#pullrequestreview-2456056327 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55690#pullrequestreview-2456423467 ✔ Last GitHub CI successful ℹ Green GitHub CI is sufficient -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 55690 From https://github.com/nodejs/node * branch refs/pull/55690/merge -> FETCH_HEAD ✔ Fetched commits as 3889b449c446..b09b94ebbf7d -------------------------------------------------------------------------------- [main 3fe086a367] build: add create release proposal action Author: RafaelGSS <[email protected]> Date: Fri Nov 1 14:52:17 2024 -0300 2 files changed, 125 insertions(+) create mode 100644 .github/workflows/create-release-proposal.yml create mode 100755 tools/actions/create-release.sh [main fc69f892e1] fixup! build: add create release proposal action Author: RafaelGSS <[email protected]> Date: Fri Nov 22 17:01:01 2024 -0300 1 file changed, 4 insertions(+), 1 deletion(-) [main 999a1c6486] fixup! fixup! build: add create release proposal action Author: RafaelGSS <[email protected]> Date: Fri Nov 22 19:49:30 2024 -0300 1 file changed, 2 insertions(+), 13 deletions(-) [main 8a5a069f13] fixup! fixup! fixup! build: add create release proposal action Author: RafaelGSS <[email protected]> Date: Fri Nov 22 20:59:00 2024 -0300 2 files changed, 8 insertions(+), 6 deletions(-) [main 6f54aa511c] Update .github/workflows/create-release-proposal.yml Author: Rafael Gonzaga <[email protected]> Date: Fri Nov 22 21:07:07 2024 -0300 1 file changed, 1 insertion(+), 1 deletion(-) ✔ Patches applied There are 5 commits in the PR. Attempting autorebase. Rebasing (2/7) Rebasing (3/7) Rebasing (4/7) Rebasing (5/7) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- build: add create release proposal actionhttps://github.com/nodejs/node/actions/runs/11988570649 |
Landed in c75aaac |
PR-URL: nodejs#55690 Refs: nodejs/security-wg#860 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #55690 Refs: nodejs/security-wg#860 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
Blocked until the following PR lands
Refs: nodejs/security-wg#860
The purpose is to have an action that will generate the release proposal (assuming the vN.x-staging is up to date). After that, I'll create a second action the keep the staging branches up-to-date.
cc: @nodejs/releasers