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

feat(git-node): use a single git push command #870

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Nov 13, 2024

I cannot see why we would prefer having several git push commands. However, I see a number of reasons to have only one, at the very end of the --promote sequence:

  • it makes it less likely a release would go half through, e.g. if the releaser has to abandon the release before the git push for whatever reason.
  • for folks watching the repo, the delay between "the proposal branch gets merged" and "the release tag is out" can be frustrating.
  • it removes the need for a --continue (cc @RafaelGSS); with this PR, you either have pushed and the only remaining command is tools/release.sh, or you haven't pushed and can start over.

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM

@aduh95
Copy link
Contributor Author

aduh95 commented Nov 13, 2024

It's probably wiser to wait for a release proposal PR to be open so this can be tested, I reckon I might be missing a .trim() call somewhere

@RafaelGSS
Copy link
Member

Right, so I'm going to land #868 without this change

@aduh95
Copy link
Contributor Author

aduh95 commented Nov 16, 2024

Just tested it with nodejs/node#55879, a .trim() call was indeed needed

@aduh95 aduh95 merged commit 82527ad into nodejs:main Nov 19, 2024
11 checks passed
@aduh95 aduh95 deleted the git-push-once branch November 19, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants