doc: update landing pr info in onboarding doc

Reword and re-organize. Only significant content change is to specifically call out the two-CTC-member-LGTM requirement for semver-major changes. PR-URL: #8344 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
nodejs · Oct 18, 2016 · e273360 · e273360
1 parent c7795ed
commit e273360
Showing 1 changed file with 25 additions and 23 deletions.
diff --git a/doc/onboarding.md b/doc/onboarding.md
@@ -116,39 +116,40 @@ onboarding session.
       * The remaining elements on the form are typically unchanged with the exception of `POST_STATUS_TO_PR`. Check that if you want a CI status indicator to be automatically inserted into the PR.
 
 
-## process for getting code in
+## Landing PRs: Overview
 
-  * the collaborator guide is a great resource: https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#technical-howto
+  * The [Collaborator Guide](https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#technical-howto) is a great resource.
 
 
-  * no one (including TSC or CTC members) pushes directly to master without review
-    * an exception is made for release commits only
+  * No one (including TSC or CTC members) pushes directly to master without review.
+    * An exception is made for release commits only.
 
 
-  * one "LGTM" is usually sufficient, except for semver-major changes
-    * the more the better
-    * semver-major (breaking) changes must be reviewed in some form by the CTC
+  * One `LGTM` is sufficient, except for semver-major changes.
+    * More than one is better.
+    * Breaking changes must be LGTM'ed by at least two CTC members.
+    * If one or more Collaborators object to a change, it should not land until
+    the objection is addressed. The options for such a situation include:
+      * Engaging those with objections to determine a viable path forward;
+      * Altering the pull request to address the objections;
+      * Escalating the discussion to the CTC using the `ctc-agenda` label. This should only be done after other options have been exhausted.
 
+  * Wait before merging non-trivial changes.
+    * 48 hours during the week and 72 hours on weekends.
+    * An example of a trivial change would be correcting the misspelling of a single word in a documentation file. This sort of change still needs to receive at least one `LGTM` but it does not need to wait 48 hours before landing.
 
-  * be sure to wait before merging non-trivial changes
-    * 48 hours for non-trivial changes, and 72 hours on weekends.
+  * **Run the PR through CI before merging!**
+    * An exception can be made for documentation-only PRs as long as it does not include the `addons.md` documentation file. (Example code from that document is extracted and built as part of the tests!)
 
-
-  * **make sure to run the PR through CI before merging!** (Except for documentation PRs)
-
-
-  * once code is ready to go in:
-    * [**See "Landing PRs"**](#landing-prs) below
-
-
-  * what if something goes wrong?
-    * ping a CTC member
+  * What if something goes wrong?
+    * Ping a CTC member.
     * `#node-dev` on freenode
-    * force-pushing to fix things after is allowed for ~10 minutes, be sure to notify people in IRC if you need to do this, but avoid it
-    * Info on PRs that don't like to apply found under [**"If `git am` fails"**](./onboarding-extras.md#if-git-am-fails).
+    * Force-pushing to fix things after is allowed for ~10 minutes. Avoid it if you can.
+      * Use `--force-with-lease` to minimize the chance of overwriting someone else's change.
+      * Post to `#node-dev` (IRC) if you force push.
 
 
-## Landing PRs
+## Landing PRs: Details
 
 * Please never use GitHub's green "Merge Pull Request" button.
   * If you do, please force-push removing the merge.
@@ -160,6 +161,7 @@ Update your `master` branch (or whichever branch you are landing on, almost alwa
 Landing a PR
 
 * if it all looks good, `curl -L 'url-of-pr.patch' | git am`
+  * If `git am` fails, see [the relevant section of the Onboarding Extras doc](./onboarding-extras.md#if-git-am-fails).
 * `git rebase -i upstream/master`
 * squash into logical commits if necessary
 * `./configure && make -j8 test` (`-j8` builds node in parallel with 8 threads. adjust to the number of cores (or processor-level threads) your processor has (or slightly more) for best results.)
@@ -171,7 +173,7 @@ Landing a PR
     * `Reviewed-By: human <email>`
       * Easiest to use `git log` then do a search
       * (`/Name` + `enter` (+ `n` as much as you need to) in vim)
-      * Only include collaborators who have commented "LGTM"
+      * Only include collaborators who have commented `LGTM`
     * `PR-URL: <full-pr-url>`
 * `git push upstream master`
     * close the original PR with "Landed in `<commit hash>`".