Skip to content

Commit 07ded7c

Browse files
TrottBethGriggs
authored andcommittedApr 16, 2019
doc: edit "Technical How-To" section of guide
Edit the "Technical How-To" section of the Collaborator Guide. Keep wording simple and direct. PR-URL: #26601 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 4274542 commit 07ded7c

File tree

1 file changed

+33
-38
lines changed

1 file changed

+33
-38
lines changed
 

‎COLLABORATOR_GUIDE.md

+33-38
Original file line numberDiff line numberDiff line change
@@ -522,18 +522,19 @@ Apply external patches:
522522
$ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
523523
```
524524

525-
If the merge fails even though recent CI runs were successful, then a 3-way
526-
merge may be required. In this case try:
525+
If the merge fails even though recent CI runs were successful, try a 3-way
526+
merge:
527527

528528
```text
529529
$ git am --abort
530530
$ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am -3 --whitespace=fix
531531
```
532-
If the 3-way merge succeeds you can proceed, but make sure to check the changes
533-
against the original PR carefully and build/test on at least one platform
534-
before landing. If the 3-way merge fails, then it is most likely that a
535-
conflicting PR has landed since the CI run and you will have to ask the author
536-
to rebase.
532+
533+
If the 3-way merge succeeds, check the results against the original pull
534+
request. Build and test on at least one platform before landing.
535+
536+
If the 3-way merge fails, then it is most likely that a conflicting pull request
537+
has landed since the CI run. You will have to ask the author to rebase.
537538

538539
Check and re-review the changes:
539540

@@ -599,52 +600,46 @@ reword 51759dc crypto: feature B
599600
fixup 7d6f433 test for feature B
600601
```
601602

602-
Save the file and close the editor. You'll be asked to enter a new
603-
commit message for that commit. This is a good moment to fix incorrect
604-
commit logs, ensure that they are properly formatted, and add
605-
`Reviewed-By` lines.
603+
Save the file and close the editor. When prompted, enter a new commit message
604+
for that commit. This is an opportunity to fix commit messages.
606605

607606
* The commit message text must conform to the [commit message guidelines][].
608607

609608
<a name="metadata"></a>
610-
* Modify the original commit message to include additional metadata regarding
611-
the change process. (The [`git node metadata`][git-node-metadata] command
612-
can generate the metadata for you.)
613-
614-
* Required: A `PR-URL:` line that references the *full* GitHub URL of the
615-
original pull request being merged so it's easy to trace a commit back to
616-
the conversation that led up to that change.
617-
* Optional: A `Fixes: X` line, where _X_ either includes the *full* GitHub URL
618-
for an issue, and/or the hash and commit message if the commit fixes
619-
a bug in a previous commit. Multiple `Fixes:` lines may be added if
620-
appropriate.
609+
* Change the original commit message to include metadata. (The
610+
[`git node metadata`][git-node-metadata] command can generate the metadata
611+
for you.)
612+
613+
* Required: A `PR-URL:` line that references the full GitHub URL of the pull
614+
request. This makes it easy to trace a commit back to the conversation that
615+
led up to that change.
616+
* Optional: A `Fixes: X` line, where _X_ is the full GitHub URL for an
617+
issue. A commit message may include more than one `Fixes:` lines.
621618
* Optional: One or more `Refs:` lines referencing a URL for any relevant
622619
background.
623-
* Required: A `Reviewed-By: Name <email>` line for yourself and any
624-
other Collaborators who have reviewed the change.
620+
* Required: A `Reviewed-By: Name <email>` line for each Collaborator who
621+
reviewed the change.
625622
* Useful for @mentions / contact list if something goes wrong in the PR.
626623
* Protects against the assumption that GitHub will be around forever.
627624

628-
Run tests (`make -j4 test` or `vcbuild test`). Even though there was a
629-
successful continuous integration run, other changes may have landed on master
630-
since then, so running the tests one last time locally is a good practice.
625+
Other changes may have landed on master since the successful CI run. As a
626+
precaution, run tests (`make -j4 test` or `vcbuild test`).
631627

632-
Validate that the commit message is properly formatted using
628+
Confirm that the commit message format is correct using
633629
[core-validate-commit](https://github.com/evanlucas/core-validate-commit).
634630

635631
```text
636632
$ git rev-list upstream/master...HEAD | xargs core-validate-commit
637633
```
638634

639-
Optional: When landing your own commits, force push the amended commit to the
640-
branch you used to open the pull request. If your branch is called `bugfix`,
641-
then the command would be `git push --force-with-lease origin master:bugfix`.
642-
Don't manually close the PR, GitHub will close it automatically later after you
643-
push it upstream, and will mark it with the purple merged status rather than the
644-
red closed status. If you close the PR before GitHub adjusts its status, it will
645-
show up as a 0 commit PR and the changed file history will be empty. Also if you
646-
push upstream before you push to your branch, GitHub will close the issue with
647-
red status so the order of operations is important.
635+
Optional: For your own commits, force push the amended commit to the pull
636+
request branch. If your branch name is `bugfix`, then: `git push
637+
--force-with-lease origin master:bugfix`. Don't close the PR. It will close
638+
after you push it upstream. It will have the purple merged status rather than
639+
the red closed status. If you close the PR before GitHub adjusts its status, it
640+
will show up as a 0 commit PR with no changed files. The order of operations is
641+
important. If you push upstream before you push to your branch, GitHub will
642+
close the issue with the red closed status.
648643

649644
Time to push it:
650645

@@ -655,7 +650,7 @@ $ git push upstream master
655650
Close the pull request with a "Landed in `<commit hash>`" comment. If
656651
your pull request shows the purple merged status then you should still
657652
add the "Landed in <commit hash>..<commit hash>" comment if you added
658-
multiple commits.
653+
more than one commit.
659654

660655
### Troubleshooting
661656

0 commit comments

Comments
 (0)