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

doc: make explicit reverting node_version.h changes #36461

Closed
wants to merge 2 commits into from

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Dec 9, 2020

Add an explicit command to revert changes to node_version.h when
cherry-picking the release commit to the master branch.

cc @nodejs/releasers

Refs: #36460
Refs: #36385

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Add an explicit command to revert changes to `node_version.h` when
cherry-picking the release commit to the `master` branch.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 9, 2020
@danielleadams
Copy link
Member

danielleadams commented Dec 9, 2020

Is there GitHub check/action that can be created to read node_version.h and check if the 1 has been flipped before merging? I suppose something that would also pass with the original PR release commit.

I follow the releases guide step by step though, so this would be helpful for me 👍🏼

@richardlau
Copy link
Member Author

Is there GitHub check/action that can be created to read node_version.h and check if the 1 has been flipped before merging? I suppose something that would also pass with the original PR release commit.

I follow the releases guide step by step though, so this would be helpful for me 👍🏼

I'm not aware if there's anything we can do to via actions to prevent merging as we're talking here about releasers cherry-picking the release commit onto the master branch which happens outside of the PR for the release (and there's no PR for the cherry-pick). Maybe something can be done in node-core-utils and the guide changed to use that? 🤷

Copy link
Member

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM

@MylesBorins
Copy link
Member

I know that @codebytere was working on the "land" portion of git node release... this seems like something that could be handled there once that bit of automation is complete

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@targos
Copy link
Member

targos commented Dec 10, 2020

Does it also work for releases that do not change the version number (and therefore do not generate conflicts)? It's usual with semver-patch LTS releases.

@richardlau
Copy link
Member Author

Does it also work for releases that do not change the version number (and therefore do not generate conflicts)? It's usual with semver-patch LTS releases.

I've pushed a small fixup for the case where there are no conflicts in node_version.h. Tested with git cherry-pick f95d7152cb9f56b3d6ca8e107a01c199b235c0df (no conflicts in node_version.h) and git cherry-pick upstream/v15.x^ (conflicts in node_version.h).

@bhaskarvilles
Copy link

bhaskarvilles commented Dec 11, 2020 via email

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 16, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 16, 2020
@github-actions
Copy link
Contributor

Landed in a150926...0b3caad

@github-actions github-actions bot closed this Dec 16, 2020
nodejs-github-bot pushed a commit that referenced this pull request Dec 16, 2020
Add an explicit command to revert changes to `node_version.h` when
cherry-picking the release commit to the `master` branch.

PR-URL: #36461
Refs: #36460
Refs: #36385
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau richardlau deleted the releases branch December 16, 2020 11:00
targos pushed a commit that referenced this pull request Dec 21, 2020
Add an explicit command to revert changes to `node_version.h` when
cherry-picking the release commit to the `master` branch.

PR-URL: #36461
Refs: #36460
Refs: #36385
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
Add an explicit command to revert changes to `node_version.h` when
cherry-picking the release commit to the `master` branch.

PR-URL: #36461
Refs: #36460
Refs: #36385
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet