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  - [Long Term Support](#long-term-support)
  12
  13 This document contains information for Collaborators of the Node.js
  14 project regarding maintaining the code, documentation and issues.
  15
  16 Collaborators should be familiar with the guidelines for new
  17 contributors in [CONTRIBUTING.md](./CONTRIBUTING.md) and also
  18 understand the project governance model as outlined in
  19 [GOVERNANCE.md](./GOVERNANCE.md).
  20
  21 ## Issues and Pull Requests
  22
  23 Courtesy should always be shown to individuals submitting issues and
  24 pull requests to the Node.js project.
  25
  26 Collaborators should feel free to take full responsibility for
  27 managing issues and pull requests they feel qualified to handle, as
  28 long as this is done while being mindful of these guidelines, the
  29 opinions of other Collaborators and guidance of the TC.
  30
  31 Collaborators may **close** any issue or pull request they believe is
  32 not relevant for the future of the Node.js project. Where this is
  33 unclear, the issue should be left open for several days to allow for
  34 additional discussion. Where this does not yield input from Node.js
  35 Collaborators or additional evidence that the issue has relevance, the
  36 issue may be closed. Remember that issues can always be re-opened if
  37 necessary.
  38
  39 ## Accepting Modifications
  40
  41 All modifications to the Node.js code and documentation should be
  42 performed via GitHub pull requests, including modifications by
  43 Collaborators and TC members.
  44
  45 All pull requests must be reviewed and accepted by a Collaborator with
  46 sufficient expertise who is able to take full responsibility for the
  47 change. In the case of pull requests proposed by an existing
  48 Collaborator, an additional Collaborator is required for sign-off.
  49
  50 In some cases, it may be necessary to summon a qualified Collaborator
  51 to a pull request for review by @-mention.
  52
  53 If you are unsure about the modification and are not prepared to take
  54 full responsibility for the change, defer to another Collaborator.
  55
  56 Before landing pull requests, sufficient time should be left for input
  57 from other Collaborators. Leave at least 48 hours during the week and
  58 72 hours over weekends to account for international time differences
  59 and work schedules. Trivial changes (e.g. those which fix minor bugs
  60 or improve performance without affecting API or causing other
  61 wide-reaching impact) may be landed after a shorter delay.
  62
  63 Where there is no disagreement amongst Collaborators, a pull request
  64 may be landed given appropriate review. Where there is discussion
  65 amongst Collaborators, consensus should be sought if possible. The
  66 lack of consensus may indicate the need to elevate discussion to the
  67 TC for resolution (see below).
  68
  69 All bugfixes require a test case which demonstrates the defect. The
  70 test should *fail* before the change, and *pass* after the change.
  71
  72 All pull requests that modify executable code should be subjected to
  73 continuous integration tests on the
  74 [project CI server](https://ci.nodejs.org/).
  75
  76 ### Involving the TC
  77
  78 Collaborators may opt to elevate pull requests or issues to the TC for
  79 discussion by assigning the ***tc-agenda*** tag. This should be done
  80 where a pull request:
  81
  82 - has a significant impact on the codebase,
  83 - is inherently controversial; or
  84 - has failed to reach consensus amongst the Collaborators who are
  85   actively participating in the discussion.
  86
  87 The TC should serve as the final arbiter where required.
  88
  89 ## Landing Pull Requests
  90
  91 Always modify the original commit message to include additional meta
  92 information regarding the change process:
  93
  94 - A `Reviewed-By: Name <email>` line for yourself and any
  95   other Collaborators who have reviewed the change.
  96 - A `PR-URL:` line that references the full GitHub URL of the original
  97   pull request being merged so it's easy to trace a commit back to the
  98   conversation that led up to that change.
  99 - A `Fixes: X` line, where _X_ is either includes the full GitHub URL
 100   for an issue, and/or the hash and commit message if the commit fixes
 101   a bug in a previous commit. Multiple `Fixes:` lines may be added if
 102   appropriate.
 103
 104 See the commit log for examples such as
 105 [this one](https://github.com/nodejs/node/commit/b636ba8186) if unsure
 106 exactly how to format your commit messages.
 107
 108 Additionally:
 109
 110 - Double check PRs to make sure the person's _full name_ and email
 111   address are correct before merging.
 112 - Except when updating dependencies, all commits should be self
 113   contained (meaning every commit should pass all tests). This makes
 114   it much easier when bisecting to find a breaking change.
 115
 116 ### Technical HOWTO
 117
 118 _Optional:_ ensure that you are not in a borked `am`/`rebase` state
 119
 120 ```text
 121 $ git am --abort
 122 $ git rebase --abort
 123 ```
 124
 125 Checkout proper target branch
 126
 127 ```text
 128 $ git checkout master
 129 ```
 130
 131 Update the tree
 132
 133 ```text
 134 $ git fetch origin
 135 $ git merge --ff-only origin/master
 136 ```
 137
 138 Apply external patches
 139
 140 ```text
 141 $ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
 142 ```
 143
 144 Check and re-review the changes
 145
 146 ```text
 147 $ git diff origin/master
 148 ```
 149
 150 Check number of commits and commit messages
 151
 152 ```text
 153 $ git log origin/master...master
 154 ```
 155
 156 If there are multiple commits that relate to the same feature or
 157 one with a feature and separate with a test for that feature,
 158 you'll need to use `squash` or `fixup`:
 159
 160 ```text
 161 $ git rebase -i origin/master
 162 ```
 163
 164 This will open a screen like this (in the default shell editor):
 165
 166 ```text
 167 pick 6928fc1 crypto: add feature A
 168 pick 8120c4c add test for feature A
 169 pick 51759dc feature B
 170 pick 7d6f433 test for feature B
 171
 172 # Rebase f9456a2..7d6f433 onto f9456a2
 173 #
 174 # Commands:
 175 #  p, pick = use commit
 176 #  r, reword = use commit, but edit the commit message
 177 #  e, edit = use commit, but stop for amending
 178 #  s, squash = use commit, but meld into previous commit
 179 #  f, fixup = like "squash", but discard this commit's log message
 180 #  x, exec = run command (the rest of the line) using shell
 181 #
 182 # These lines can be re-ordered; they are executed from top to bottom.
 183 #
 184 # If you remove a line here THAT COMMIT WILL BE LOST.
 185 #
 186 # However, if you remove everything, the rebase will be aborted.
 187 #
 188 # Note that empty commits are commented out
 189 ```
 190
 191 Replace a couple of `pick`s with `fixup` to squash them into a
 192 previous commit:
 193
 194 ```text
 195 pick 6928fc1 crypto: add feature A
 196 fixup 8120c4c add test for feature A
 197 pick 51759dc feature B
 198 fixup 7d6f433 test for feature B
 199 ```
 200
 201 Replace `pick` with `reword` to change the commit message:
 202
 203 ```text
 204 reword 6928fc1 crypto: add feature A
 205 fixup 8120c4c add test for feature A
 206 reword 51759dc feature B
 207 fixup 7d6f433 test for feature B
 208 ```
 209
 210 Save the file and close the editor. You'll be asked to enter a new
 211 commit message for that commit. This is a good moment to fix incorrect
 212 commit logs, ensure that they are properly formatted, and add
 213 `Reviewed-By` lines.
 214
 215 Time to push it:
 216
 217 ```text
 218 $ git push origin master
 219 ```
 220
 221 ### I Just Made a Mistake
 222
 223 With `git`, there's a way to override remote trees by force pushing
 224 (`git push -f`). This should generally be seen as forbidden (since
 225 you're rewriting history on a repository other people are working
 226 against) but is allowed for simpler slip-ups such as typos in commit
 227 messages. However, you are only allowed to force push to any Node.js
 228 branch within 10 minutes from your original push. If someone else
 229 pushes to the branch or the 10 minute period passes, consider the
 230 commit final.
 231
 232 ### Long Term Support
 233
 234 #### What is LTS?
 235
 236 Long Term Support (often referred to as *LTS*) guarantees application developers
 237 a 30 month support cycle with specific versions of Node.js.
 238
 239 You can find more information [in the full LTS plan](https://github.com/nodejs/lts#lts-plan).
 240
 241 #### How does LTS work?
 242
 243 Once a stable branch enters LTS, changes in that branch are limited to bug
 244 fixes, security updates, possible npm updates, documentation updates, and
 245 certain performance improvements that can be demonstrated to not break existing
 246 applications. Semver-minor changes are only permitted if required for bug fixes
 247 and then only on a case-by-case basis with LTS WG and possibly Core Technical
 248 Committee (CTC) review. Semver-major changes are permitted only if required for
 249 security related fixes.
 250
 251 Once a stable branch moves into Maintenance mode, only **critical** bugs,
 252 **critical** security fixes, and documentation updates will be permitted.
 253
 254 #### Landing semver-minor commits in LTS
 255
 256 The default policy is to not land semver-minor or higher commits in any LTS
 257 branch. However, the LTS WG or CTC can evaluate any individual semver-minor
 258 commit and decide whether a special exception ought to be made. It is
 259 expected that such exceptions would be evaluated, in part, on the scope
 260 and impact of the changes on the code, the risk to ecosystem stability
 261 incurred by accepting the change, and the expected benefit that landing the
 262 commit will have for the ecosystem.
 263
 264 Any collaborator who feels a semver-minor commit should be landed in an LTS
 265 branch should attach the `lts-agenda` label to the pull request. The LTS WG
 266 will discuss the issue and, if necessary, will escalate the issue up to the
 267 CTC for further discussion.
 268
 269 #### How are LTS Branches Managed?
 270
 271 There are currently three LTS branches: `v4.x`, `v0.10`, and `v0.12`. Each
 272 of these is paired with a "staging" branch: `v4.x-staging`, `v0.10-staging`,
 273 and `v0.12-staging`.
 274
 275 As commits land in `master`, they are cherry-picked back to each staging
 276 branch as appropriate. If the commit applies only to the LTS branch, the
 277 PR must be opened against the *staging* branch. Commits are selectively
 278 pulled from the staging branch into the LTS branch only when a release is
 279 being prepared and may be pulled into the LTS branch in a different order
 280 than they were landed in staging.
 281
 282 Any collaborator may land commits into a staging branch, but only the release
 283 team should land commits into the LTS branch while preparing a new
 284 LTS release.
 285
 286 #### How can I help?
 287
 288 When you send your pull request, consider including information about
 289 whether your change is breaking. If you think your patch can be backported,
 290 please feel free to include that information in the PR thread.
 291
 292 Several LTS related issue and PR labels have been provided:
 293
 294 * `lts-watch-v4.x` - tells the LTS WG that the issue/PR needs to be considered
 295   for landing in the `v4.x-staging` branch.
 296 * `lts-watch-v0.10` - tells the LTS WG that the issue/PR needs to be considered
 297   for landing in the `v0.10-staging` branch.
 298 * `lts-watch-v0.12` - tells the LTS WG that the issue/PR needs to be considered
 299   for landing in the `v0.12-staging` branch.
 300 * `land-on-v4.x` - tells the release team that the commit should be landed
 301   in a future v4.x release
 302 * `land-on-v0.10` - tells the release team that the commit should be landed
 303   in a future v0.10 release
 304 * `land-on-v0.12` - tells the release team that the commit should be landed
 305   in a future v0.12 release
 306
 307 Any collaborator can attach these labels to any PR/issue. As commits are
 308 landed into the staging branches, the `lts-watch-` label will be removed.
 309 Likewise, as commits are landed in a LTS release, the `land-on-` label will
 310 be removed.
 311
 312 Collaborators are encouraged to help the LTS WG by attaching the appropriate
 313 `lts-watch-` label to any PR that may impact an LTS release.
 314
 315 #### How is an LTS release cut?
 316
 317 When the LTS working group determines that a new LTS release is required,
 318 selected commits will be picked from the staging branch to be included in the
 319 release. This process of making a release will be a collaboration between the
 320 LTS working group and the Release team.