1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

doc: update landing pr info in onboarding doc

Browse Source 
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: https://github.com/nodejs/node/pull/8344
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
This commit is contained in:
Rich Trott 2016-08-30 11:20:55 -07:00
parent a133b775e3
commit 2501a38b79
1 changed files with 25 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
doc/onboarding.md
View File

@ -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>`".