COLLABORATOR_GUIDE.md

   1 # Node.js Collaborator Guide
   2
   3 **Contents**
   4
   5 * [Issues and Pull Requests](#issues-and-pull-requests)
   6 * [Accepting Modifications](#accepting-modifications)
   7  - [Involving the TC](#involving-the-tc)
   8 * [Landing Pull Requests](#landing-pull-requests)
   9  - [Technical HOWTO](#technical-howto)
  10  - [I Just Made a Mistake](#i-just-made-a-mistake)
  11
  12 This document contains information for Collaborators of the Node.js
  13 project regarding maintaining the code, documentation and issues.
  14
  15 Collaborators should be familiar with the guidelines for new
  16 contributors in [CONTRIBUTING.md](./CONTRIBUTING.md) and also
  17 understand the project governance model as outlined in
  18 [GOVERNANCE.md](./GOVERNANCE.md).
  19
  20 ## Issues and Pull Requests
  21
  22 Courtesy should always be shown to individuals submitting issues and
  23 pull requests to the Node.js project.
  24
  25 Collaborators should feel free to take full responsibility for
  26 managing issues and pull requests they feel qualified to handle, as
  27 long as this is done while being mindful of these guidelines, the
  28 opinions of other Collaborators and guidance of the TC.
  29
  30 Collaborators may **close** any issue or pull request they believe is
  31 not relevant for the future of the Node.js project. Where this is
  32 unclear, the issue should be left open for several days to allow for
  33 additional discussion. Where this does not yield input from Node.js
  34 Collaborators or additional evidence that the issue has relevance, the
  35 issue may be closed. Remember that issues can always be re-opened if
  36 necessary.
  37
  38 ## Accepting Modifications
  39
  40 All modifications to the Node.js code and documentation should be
  41 performed via GitHub pull requests, including modifications by
  42 Collaborators and TC members.
  43
  44 All pull requests must be reviewed and accepted by a Collaborator with
  45 sufficient expertise who is able to take full responsibility for the
  46 change. In the case of pull requests proposed by an existing
  47 Collaborator, an additional Collaborator is required for sign-off.
  48
  49 In some cases, it may be necessary to summon a qualified Collaborator
  50 to a pull request for review by @-mention.
  51
  52 If you are unsure about the modification and are not prepared to take
  53 full responsibility for the change, defer to another Collaborator.
  54
  55 Before landing pull requests, sufficient time should be left for input
  56 from other Collaborators. Leave at least 48 hours during the week and
  57 72 hours over weekends to account for international time differences
  58 and work schedules. Trivial changes (e.g. those which fix minor bugs
  59 or improve performance without affecting API or causing other
  60 wide-reaching impact) may be landed after a shorter delay.
  61
  62 Where there is no disagreement amongst Collaborators, a pull request
  63 may be landed given appropriate review. Where there is discussion
  64 amongst Collaborators, consensus should be sought if possible. The
  65 lack of consensus may indicate the need to elevate discussion to the
  66 TC for resolution (see below).
  67
  68 All bugfixes require a test case which demonstrates the defect. The
  69 test should *fail* before the change, and *pass* after the change.
  70
  71 All pull requests that modify executable code should be subjected to
  72 continuous integration tests on the
  73 [project CI server](https://jenkins-iojs.nodesource.com/).
  74
  75 ### Involving the TC
  76
  77 Collaborators may opt to elevate pull requests or issues to the TC for
  78 discussion by assigning the ***tc-agenda*** tag. This should be done
  79 where a pull request:
  80
  81 - has a significant impact on the codebase,
  82 - is inherently controversial; or
  83 - has failed to reach consensus amongst the Collaborators who are
  84   actively participating in the discussion.
  85
  86 The TC should serve as the final arbiter where required.
  87
  88 ## Landing Pull Requests
  89
  90 Always modify the original commit message to include additional meta
  91 information regarding the change process:
  92
  93 - A `Reviewed-By: Name <email>` line for yourself and any
  94   other Collaborators who have reviewed the change.
  95 - A `PR-URL:` line that references the full GitHub URL of the original
  96   pull request being merged so it's easy to trace a commit back to the
  97   conversation that led up to that change.
  98 - A `Fixes: X` line, where _X_ is either includes the full GitHub URL
  99   for an issue, and/or the hash and commit message if the commit fixes
 100   a bug in a previous commit. Multiple `Fixes:` lines may be added if
 101   appropriate.
 102
 103 See the commit log for examples such as
 104 [this one](https://github.com/nodejs/node/commit/b636ba8186) if unsure
 105 exactly how to format your commit messages.
 106
 107 Additionally:
 108
 109 - Double check PRs to make sure the person's _full name_ and email
 110   address are correct before merging.
 111 - Except when updating dependencies, all commits should be self
 112   contained (meaning every commit should pass all tests). This makes
 113   it much easier when bisecting to find a breaking change.
 114
 115 ### Technical HOWTO
 116
 117 _Optional:_ ensure that you are not in a borked `am`/`rebase` state
 118
 119 ```text
 120 $ git am --abort
 121 $ git rebase --abort
 122 ```
 123
 124 Checkout proper target branch
 125
 126 ```text
 127 $ git checkout master
 128 ```
 129
 130 Update the tree
 131
 132 ```text
 133 $ git fetch origin
 134 $ git merge --ff-only origin/master
 135 ```
 136
 137 Apply external patches
 138
 139 ```text
 140 $ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
 141 ```
 142
 143 Check and re-review the changes
 144
 145 ```text
 146 $ git diff origin/master
 147 ```
 148
 149 Check number of commits and commit messages
 150
 151 ```text
 152 $ git log origin/master...master
 153 ```
 154
 155 If there are multiple commits that relate to the same feature or
 156 one with a feature and separate with a test for that feature,
 157 you'll need to use `squash` or `fixup`:
 158
 159 ```text
 160 $ git rebase -i origin/master
 161 ```
 162
 163 This will open a screen like this (in the default shell editor):
 164
 165 ```text
 166 pick 6928fc1 crypto: add feature A
 167 pick 8120c4c add test for feature A
 168 pick 51759dc feature B
 169 pick 7d6f433 test for feature B
 170
 171 # Rebase f9456a2..7d6f433 onto f9456a2
 172 #
 173 # Commands:
 174 #  p, pick = use commit
 175 #  r, reword = use commit, but edit the commit message
 176 #  e, edit = use commit, but stop for amending
 177 #  s, squash = use commit, but meld into previous commit
 178 #  f, fixup = like "squash", but discard this commit's log message
 179 #  x, exec = run command (the rest of the line) using shell
 180 #
 181 # These lines can be re-ordered; they are executed from top to bottom.
 182 #
 183 # If you remove a line here THAT COMMIT WILL BE LOST.
 184 #
 185 # However, if you remove everything, the rebase will be aborted.
 186 #
 187 # Note that empty commits are commented out
 188 ```
 189
 190 Replace a couple of `pick`s with `fixup` to squash them into a
 191 previous commit:
 192
 193 ```text
 194 pick 6928fc1 crypto: add feature A
 195 fixup 8120c4c add test for feature A
 196 pick 51759dc feature B
 197 fixup 7d6f433 test for feature B
 198 ```
 199
 200 Replace `pick` with `reword` to change the commit message:
 201
 202 ```text
 203 reword 6928fc1 crypto: add feature A
 204 fixup 8120c4c add test for feature A
 205 reword 51759dc feature B
 206 fixup 7d6f433 test for feature B
 207 ```
 208
 209 Save the file and close the editor. You'll be asked to enter a new
 210 commit message for that commit. This is a good moment to fix incorrect
 211 commit logs, ensure that they are properly formatted, and add
 212 `Reviewed-By` lines.
 213
 214 Time to push it:
 215
 216 ```text
 217 $ git push origin master
 218 ```
 219
 220 ### I Just Made a Mistake
 221
 222 With `git`, there's a way to override remote trees by force pushing
 223 (`git push -f`). This should generally be seen as forbidden (since
 224 you're rewriting history on a repository other people are working
 225 against) but is allowed for simpler slip-ups such as typos in commit
 226 messages. However, you are only allowed to force push to any Node.js
 227 branch within 10 minutes from your original push. If someone else
 228 pushes to the branch or the 10 minute period passes, consider the
 229 commit final.