Skip to content

Commit 058cf43

Browse files
Trotttargos
authored andcommitted
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 892282d commit 058cf43

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
@@ -464,18 +464,19 @@ Apply external patches:
464464
$ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
465465
```
466466

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

470470
```text
471471
$ git am --abort
472472
$ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am -3 --whitespace=fix
473473
```
474-
If the 3-way merge succeeds you can proceed, but make sure to check the changes
475-
against the original PR carefully and build/test on at least one platform
476-
before landing. If the 3-way merge fails, then it is most likely that a
477-
conflicting PR has landed since the CI run and you will have to ask the author
478-
to rebase.
474+
475+
If the 3-way merge succeeds, check the results against the original pull
476+
request. Build and test on at least one platform before landing.
477+
478+
If the 3-way merge fails, then it is most likely that a conflicting pull request
479+
has landed since the CI run. You will have to ask the author to rebase.
479480

480481
Check and re-review the changes:
481482

@@ -541,52 +542,46 @@ reword 51759dc crypto: feature B
541542
fixup 7d6f433 test for feature B
542543
```
543544

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

549548
* The commit message text must conform to the [commit message guidelines][].
550549

551550
<a name="metadata"></a>
552-
* Modify the original commit message to include additional metadata regarding
553-
the change process. (The [`git node metadata`][git-node-metadata] command
554-
can generate the metadata for you.)
555-
556-
* Required: A `PR-URL:` line that references the *full* GitHub URL of the
557-
original pull request being merged so it's easy to trace a commit back to
558-
the conversation that led up to that change.
559-
* Optional: A `Fixes: X` line, where _X_ either includes the *full* GitHub URL
560-
for an issue, and/or the hash and commit message if the commit fixes
561-
a bug in a previous commit. Multiple `Fixes:` lines may be added if
562-
appropriate.
551+
* Change the original commit message to include metadata. (The
552+
[`git node metadata`][git-node-metadata] command can generate the metadata
553+
for you.)
554+
555+
* Required: A `PR-URL:` line that references the full GitHub URL of the pull
556+
request. This makes it easy to trace a commit back to the conversation that
557+
led up to that change.
558+
* Optional: A `Fixes: X` line, where _X_ is the full GitHub URL for an
559+
issue. A commit message may include more than one `Fixes:` lines.
563560
* Optional: One or more `Refs:` lines referencing a URL for any relevant
564561
background.
565-
* Required: A `Reviewed-By: Name <email>` line for yourself and any
566-
other Collaborators who have reviewed the change.
562+
* Required: A `Reviewed-By: Name <email>` line for each Collaborator who
563+
reviewed the change.
567564
* Useful for @mentions / contact list if something goes wrong in the PR.
568565
* Protects against the assumption that GitHub will be around forever.
569566

570-
Run tests (`make -j4 test` or `vcbuild test`). Even though there was a
571-
successful continuous integration run, other changes may have landed on master
572-
since then, so running the tests one last time locally is a good practice.
567+
Other changes may have landed on master since the successful CI run. As a
568+
precaution, run tests (`make -j4 test` or `vcbuild test`).
573569

574-
Validate that the commit message is properly formatted using
570+
Confirm that the commit message format is correct using
575571
[core-validate-commit](https://github.com/evanlucas/core-validate-commit).
576572

577573
```text
578574
$ git rev-list upstream/master...HEAD | xargs core-validate-commit
579575
```
580576

581-
Optional: When landing your own commits, force push the amended commit to the
582-
branch you used to open the pull request. If your branch is called `bugfix`,
583-
then the command would be `git push --force-with-lease origin master:bugfix`.
584-
Don't manually close the PR, GitHub will close it automatically later after you
585-
push it upstream, and will mark it with the purple merged status rather than the
586-
red closed status. If you close the PR before GitHub adjusts its status, it will
587-
show up as a 0 commit PR and the changed file history will be empty. Also if you
588-
push upstream before you push to your branch, GitHub will close the issue with
589-
red status so the order of operations is important.
577+
Optional: For your own commits, force push the amended commit to the pull
578+
request branch. If your branch name is `bugfix`, then: `git push
579+
--force-with-lease origin master:bugfix`. Don't close the PR. It will close
580+
after you push it upstream. It will have the purple merged status rather than
581+
the red closed status. If you close the PR before GitHub adjusts its status, it
582+
will show up as a 0 commit PR with no changed files. The order of operations is
583+
important. If you push upstream before you push to your branch, GitHub will
584+
close the issue with the red closed status.
590585

591586
Time to push it:
592587

@@ -597,7 +592,7 @@ $ git push upstream master
597592
Close the pull request with a "Landed in `<commit hash>`" comment. If
598593
your pull request shows the purple merged status then you should still
599594
add the "Landed in <commit hash>..<commit hash>" comment if you added
600-
multiple commits.
595+
more than one commit.
601596

602597
### Troubleshooting
603598

0 commit comments

Comments
 (0)