From a674c6f65fc83b1806fb9bea22da924e262ffce7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 23 Dec 2021 13:28:10 +0100 Subject: [PATCH 1/3] doc: make pull-request guide default branch agnostic Refs: https://github.com/nodejs/node/issues/33864 --- doc/guides/contributing/pull-requests.md | 76 +++++++++++------------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index 228b9839ca1f8b..fec84beeceb81b 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -71,18 +71,18 @@ it's time to create a fork. Fork the project [on GitHub](https://github.com/nodejs/node) and clone your fork locally. -```text -$ git clone git@github.com:username/node.git -$ cd node -$ git remote add upstream https://github.com/nodejs/node.git -$ git fetch upstream +```sh +git clone git@github.com:username/node.git +cd node +git remote add upstream https://github.com/nodejs/node.git +git fetch upstream ``` Configure `git` so that it knows who you are: -```text -$ git config user.name "J. Random User" -$ git config user.email "j.random.user@example.com" +```sh +git config user.name "J. Random User" +git config user.email "j.random.user@example.com" ``` You can use any name/email address you prefer here. We only use the @@ -98,10 +98,10 @@ make sure this local email is also added to your As a best practice to keep your development environment as organized as possible, create local branches to work within. These should also be created -directly off of the `master` branch. +directly off of the upstream default branch. ```text -$ git checkout -b my-branch -t upstream/master +$ git checkout -b my-branch -t upstream/HEAD ``` ## The process of making changes @@ -218,13 +218,12 @@ As a best practice, once you have committed your changes, it is a good idea to use `git rebase` (not `git merge`) to synchronize your work with the main repository. -```text -$ git fetch upstream -$ git rebase upstream/master +```sh +git fetch upstream HEAD +git rebase FETCH_HEAD ``` -This ensures that your working branch has the latest changes from `nodejs/node` -master. +This ensures that your working branch has the latest changes from `nodejs/node`. ### Step 6: Test @@ -261,8 +260,8 @@ Once you are sure your commits are ready to go, with passing tests and linting, begin the process of opening a pull request by pushing your working branch to your fork on GitHub. -```text -$ git push origin my-branch +```sh +git push origin my-branch ``` ### Step 8: Opening the pull request @@ -290,33 +289,26 @@ To make changes to an existing pull request, make the changes to your local branch, add a new commit with those changes, and push those to your fork. GitHub will automatically update the pull request. -```text -$ git add my/changed/files -$ git commit -$ git push origin my-branch +```sh +git add my/changed/files +git commit +git push origin my-branch ``` -It is also frequently necessary to synchronize your pull request with other -changes that have landed in `master` by using `git rebase`: +If a git conflict arises, it is necessary to synchronize your branch with other +changes that have landed upstream by using `git rebase`: -```text -$ git fetch --all -$ git rebase upstream/master -$ git push --force-with-lease origin my-branch +```sh +git fetch upstream HEAD +git rebase FETCH_HEAD +git push --force-with-lease origin my-branch ``` **Important:** The `git push --force-with-lease` command is one of the few ways -to delete history in `git`. Before you use it, make sure you understand the -risks. If in doubt, you can always ask for guidance in the pull request. - -If you happen to make a mistake in any of your commits, do not worry. You can -amend the last commit (for example if you want to change the commit log). - -```text -$ git add any/changed/files -$ git commit --amend -$ git push --force-with-lease origin my-branch -``` +to delete history in `git`. It also complicates the review process, as it won't +allow reviewers to get a quick glance on what changed. Before you use it, make +sure you understand the risks. If in doubt, you can always ask for guidance in +the pull request. There are a number of more advanced mechanisms for managing commits using `git rebase` that can be used, but are beyond the scope of this guide. @@ -356,10 +348,10 @@ your pull request waiting longer than you expect, see the When a collaborator lands your pull request, they will post a comment to the pull request page mentioning the commit(s) it -landed as. GitHub often shows the pull request as `Closed` at this +landed as. GitHub may show the pull request as `Closed` at this point, but don't worry. If you look at the branch you raised your -pull request against (probably `master`), you should see a commit with -your name on it. Congratulations and thanks for your contribution! +pull request against, you should see a commit with your name on it. +Congratulations and thanks for your contribution! ## Reviewing pull requests @@ -542,7 +534,7 @@ For the size of "one logical change", [0b5191f](https://github.com/nodejs/node/commit/0b5191f15d0f311c804d542b67e2e922d98834f8) can be a good example. It touches the implementation, the documentation, and the tests, but is still one logical change. All tests should always pass -when each individual commit lands on the master branch. +when each individual commit lands on one of the `nodejs/node` branches. ### Getting approvals for your pull request From adb7ed361cd5339652cf03bab9b207978845ed9f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 26 Dec 2021 20:39:30 +0100 Subject: [PATCH 2/3] Use `text` instead of `sh` --- doc/guides/contributing/pull-requests.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index fec84beeceb81b..aa67a62c0ebb7a 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -71,7 +71,7 @@ it's time to create a fork. Fork the project [on GitHub](https://github.com/nodejs/node) and clone your fork locally. -```sh +```text git clone git@github.com:username/node.git cd node git remote add upstream https://github.com/nodejs/node.git @@ -80,7 +80,7 @@ git fetch upstream Configure `git` so that it knows who you are: -```sh +```text git config user.name "J. Random User" git config user.email "j.random.user@example.com" ``` @@ -101,7 +101,7 @@ possible, create local branches to work within. These should also be created directly off of the upstream default branch. ```text -$ git checkout -b my-branch -t upstream/HEAD +git checkout -b my-branch -t upstream/HEAD ``` ## The process of making changes @@ -149,8 +149,8 @@ commits any single pull request may have, and many contributors find it easier to review changes that are split across multiple commits. ```text -$ git add my/changed/files -$ git commit +git add my/changed/files +git commit ``` Multiple commits often get squashed when they are landed. See the @@ -218,7 +218,7 @@ As a best practice, once you have committed your changes, it is a good idea to use `git rebase` (not `git merge`) to synchronize your work with the main repository. -```sh +```text git fetch upstream HEAD git rebase FETCH_HEAD ``` @@ -241,7 +241,7 @@ Before submitting your changes in a pull request, always run the full Node.js test suite. To run the tests (including code linting) on Unix / macOS: ```text -$ ./configure && make -j4 test +./configure && make -j4 test ``` And on Windows: @@ -260,7 +260,7 @@ Once you are sure your commits are ready to go, with passing tests and linting, begin the process of opening a pull request by pushing your working branch to your fork on GitHub. -```sh +```text git push origin my-branch ``` @@ -289,7 +289,7 @@ To make changes to an existing pull request, make the changes to your local branch, add a new commit with those changes, and push those to your fork. GitHub will automatically update the pull request. -```sh +```text git add my/changed/files git commit git push origin my-branch @@ -298,7 +298,7 @@ git push origin my-branch If a git conflict arises, it is necessary to synchronize your branch with other changes that have landed upstream by using `git rebase`: -```sh +```text git fetch upstream HEAD git rebase FETCH_HEAD git push --force-with-lease origin my-branch From bace3a8a90a3d25ca969eaef4113025c3225c9af Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 26 Dec 2021 22:05:08 +0100 Subject: [PATCH 3/3] Update doc/guides/contributing/pull-requests.md Co-authored-by: Rich Trott --- doc/guides/contributing/pull-requests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index aa67a62c0ebb7a..95024008645916 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -348,7 +348,7 @@ your pull request waiting longer than you expect, see the When a collaborator lands your pull request, they will post a comment to the pull request page mentioning the commit(s) it -landed as. GitHub may show the pull request as `Closed` at this +landed as. GitHub might show the pull request as `Closed` at this point, but don't worry. If you look at the branch you raised your pull request against, you should see a commit with your name on it. Congratulations and thanks for your contribution!