doc: reorganize COLLABORATOR_GUIDE.md

Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benedikt Meurer <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>

Author: Trott

COLLABORATOR_GUIDE.md

@@ -371,60 +371,45 @@ The TSC should serve as the final arbiter where required.
     * If more than one author has contributed to the PR, keep the most recent
       author when squashing.
 
-Always modify the original commit message to include additional meta
-information regarding the change process:
-
-- A `PR-URL:` line that references the *full* GitHub URL of the original
-  pull request being merged so it's easy to trace a commit back to the
-  conversation that led up to that change.
-- A `Fixes: X` line, where _X_ either includes the *full* GitHub URL
-  for an issue, and/or the hash and commit message if the commit fixes
-  a bug in a previous commit. Multiple `Fixes:` lines may be added if
-  appropriate.
-- A `Refs:` line referencing a URL for any relevant background.
-- A `Reviewed-By: Name <email>` line for yourself and any
-  other Collaborators who have reviewed the change.
-  - Useful for @mentions / contact list if something goes wrong in the PR.
-  - Protects against the assumption that GitHub will be around forever.
-
 Review the commit message to ensure that it adheres to the guidelines outlined
 in the [contributing](./CONTRIBUTING.md#commit-message-guidelines) guide.
 
+Add all necessary [metadata](#metadata) to commit messages before landing.
+
 See the commit log for examples such as
 [this one](https://github.com/nodejs/node/commit/b636ba8186) if unsure
 exactly how to format your commit messages.
 
 Additionally:
 - Double check PRs to make sure the person's _full name_ and email
   address are correct before merging.
-- Except when updating dependencies, all commits should be self
-  contained (meaning every commit should pass all tests). This makes
-  it much easier when bisecting to find a breaking change.
+- All commits should be self-contained (meaning every commit should pass all
+  tests). This makes it much easier when bisecting to find a breaking change.
 
 ### Technical HOWTO
 
-Clear any `am`/`rebase` that may already be underway.
+Clear any `am`/`rebase` that may already be underway:
 
 ```text
 $ git am --abort
 $ git rebase --abort
 ```
 
-Checkout proper target branch
+Checkout proper target branch:
 
 ```text
 $ git checkout master
 ```
 
 Update the tree (assumes your repo is set up as detailed in
-[CONTRIBUTING.md](CONTRIBUTING.md#step-1-fork))
+[CONTRIBUTING.md](CONTRIBUTING.md#step-1-fork)):
 
 ```text
 $ git fetch upstream
 $ git merge --ff-only upstream/master
 ```
 
-Apply external patches
+Apply external patches:
 
 ```text
 $ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
@@ -442,21 +427,19 @@ against the original PR carefully and build/test on at least one platform
 before landing. If the 3-way merge fails, then it is most likely that a conflicting
 PR has landed since the CI run and you will have to ask the author to rebase.
 
-Check and re-review the changes
+Check and re-review the changes:
 
 ```text
 $ git diff upstream/master
 ```
 
-Check number of commits and commit messages
+Check number of commits and commit messages:
 
 ```text
 $ git log upstream/master...master
 ```
 
-If there are multiple commits that relate to the same feature or
-one with a feature and separate with a test for that feature,
-you'll need to use `squash` or `fixup`:
+Squash commits and add metadata:
 
 ```text
 $ git rebase -i upstream/master
@@ -512,9 +495,29 @@ Save the file and close the editor. You'll be asked to enter a new
 commit message for that commit. This is a good moment to fix incorrect
 commit logs, ensure that they are properly formatted, and add
 `Reviewed-By` lines.
+
 * The commit message text must conform to the
 [commit message guidelines](./CONTRIBUTING.md#commit-message-guidelines).
 
+<a name="metadata"></a>
+* Modify the original commit message to include additional metadata regarding
+  the change process. (If you use Chrome or Edge, [`node-review`][] fetches
+  the metadata for you.)
+
+  * Required: A `PR-URL:` line that references the *full* GitHub URL of the
+    original pull request being merged so it's easy to trace a commit back to
+    the conversation that led up to that change.
+  * Optional: A `Fixes: X` line, where _X_ either includes the *full* GitHub URL
+    for an issue, and/or the hash and commit message if the commit fixes
+    a bug in a previous commit. Multiple `Fixes:` lines may be added if
+    appropriate.
+  * Optional: One or more `Refs:` lines referencing a URL for any relevant
+    background.
+  * Required: A `Reviewed-By: Name <email>` line for yourself and any
+    other Collaborators who have reviewed the change.
+    * Useful for @mentions / contact list if something goes wrong in the PR.
+    * Protects against the assumption that GitHub will be around forever.
+
 Run tests (`make -j4 test` or `vcbuild test`). Even though there was a
 successful continuous integration run, other changes may have landed on master
 since then, so running the tests one last time locally is a good practice.
@@ -678,3 +681,4 @@ LTS working group and the Release team.
 [Stability Index]: doc/api/documentation.md#stability-index
 [Enhancement Proposal]: https://github.com/nodejs/node-eps
 [git-username]: https://help.github.com/articles/setting-your-username-in-git/
+[`node-review`]: https://github.com/evanlucas/node-review

doc/onboarding.md

@@ -158,7 +158,7 @@ onboarding session.
   * After one or two approvals, land the PR.
     * Be sure to add the `PR-URL: <full-pr-url>` and appropriate `Reviewed-By:` metadata!
     * [`core-validate-commit`][] helps a lot with this – install and use it if you can!
-    * If you use Chrome, [`node-review`][] fetches the metadata for you
+    * If you use Chrome or Edge, [`node-review`][] fetches the metadata for you.
 
 ## Final notes