-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
base: main
Are you sure you want to change the base?
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 |
|
||
- name: Start git node release prepare | ||
run: | | ||
./tools/actions/create-release.sh "${RELEASE_DATE}" '${{ inputs.release-line }}' |
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.
./tools/actions/create-release.sh "${RELEASE_DATE}" '${{ inputs.release-line }}' | |
./tools/actions/create-release.sh "${RELEASE_DATE}" '${RELEASE_LINE}' |
Following best practices, inputs shouldn't be directly in bash expressions. Instead, story the input as a variable and access it like above
452c3c2
to
b055972
Compare
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