-
-
Notifications
You must be signed in to change notification settings - Fork 32.9k
doc: make pull-request guide default branch agnostic #41299
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
Merged
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,17 +72,17 @@ Fork the project [on GitHub](https://github.com/nodejs/node) and clone your fork | |
locally. | ||
|
||
```text | ||
$ git clone [email protected]:username/node.git | ||
$ cd node | ||
$ git remote add upstream https://github.com/nodejs/node.git | ||
$ git fetch upstream | ||
git clone [email protected]: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 "[email protected]" | ||
git config user.name "J. Random User" | ||
git config user.email "[email protected]" | ||
``` | ||
|
||
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 | ||
|
@@ -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 | ||
|
@@ -219,12 +219,11 @@ to use `git rebase` (not `git merge`) to synchronize your work with the main | |
repository. | ||
|
||
```text | ||
$ git fetch upstream | ||
$ git rebase upstream/master | ||
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 | ||
|
||
|
@@ -242,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: | ||
|
@@ -262,7 +261,7 @@ begin the process of opening a pull request by pushing your working branch to | |
your fork on GitHub. | ||
|
||
```text | ||
$ git push origin my-branch | ||
git push origin my-branch | ||
``` | ||
|
||
### Step 8: Opening the pull request | ||
|
@@ -291,32 +290,25 @@ 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 | ||
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 | ||
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 | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.