Skip to content
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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

RafaelGSS
Copy link
Member

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

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Nov 2, 2024
@RafaelGSS RafaelGSS added the blocked PRs that are blocked by other issues or PRs. label Nov 2, 2024
.github/workflows/create-release-proposal.yml Outdated Show resolved Hide resolved
.github/workflows/create-release-proposal.yml Outdated Show resolved Hide resolved
.github/workflows/create-release-proposal.yml Outdated Show resolved Hide resolved
.github/workflows/create-release-proposal.yml Outdated Show resolved Hide resolved
.github/workflows/create-release-proposal.yml Outdated Show resolved Hide resolved
# 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
Copy link
Contributor

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

Copy link
Member Author

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

Copy link
Contributor

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

Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Member Author

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

Copy link
Contributor

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.

Copy link
Member Author

@RafaelGSS RafaelGSS Nov 13, 2024

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.

@RafaelGSS RafaelGSS removed the blocked PRs that are blocked by other issues or PRs. label Nov 5, 2024
@RafaelGSS
Copy link
Member Author

Uh, just found the action is broken. I will fix it https://github.com/nodejs/node/actions/runs/11804669470

Co-authored-by: Antoine du Hamel <[email protected]>
Co-authored-by: Aviv Keller <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>

- name: Start git node release prepare
run: |
./tools/actions/create-release.sh "${RELEASE_DATE}" '${{ inputs.release-line }}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
./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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants