3
3
** Contents**
4
4
5
5
* [ Issues and Pull Requests] ( #issues-and-pull-requests )
6
+ - [ Managing Issues and Pull Requests] ( #managing-issues-and-pull-requests )
7
+ - [ Welcoming First-Time Contributiors] ( #welcoming-first-time-contributiors )
8
+ - [ Closing Issues and Pull Requests] ( #closing-issues-and-pull-requests )
6
9
* [ Accepting Modifications] ( #accepting-modifications )
7
- - [ Useful CI Jobs] ( #useful-ci-jobs )
8
- - [ Internal vs. Public API] ( #internal-vs-public-api )
9
- - [ Breaking Changes] ( #breaking-changes )
10
- - [ Deprecations] ( #deprecations )
11
- - [ Involving the TSC] ( #involving-the-tsc )
10
+ - [ Code Reviews and Consensus Seeking] ( #code-reviews-and-consensus-seeking )
11
+ - [ Waiting for Approvals] ( #waiting-for-approvals )
12
+ - [ Testing and CI] ( #testing-and-ci )
13
+ - [ Useful CI Jobs] ( #useful-ci-jobs )
14
+ - [ Internal vs. Public API] ( #internal-vs-public-api )
15
+ - [ Breaking Changes] ( #breaking-changes )
16
+ - [ Breaking Changes and Deprecations] ( #breaking-changes-and-deprecations )
17
+ - [ Breaking Changes to Internal Elements] ( #breaking-changes-to-internal-elements )
18
+ - [ When Breaking Changes Actually Break Things] ( #when-breaking-changes-actually-break-things )
19
+ - [ Reverting commits] ( #reverting-commits )
20
+ - [ Introducing New Modules] ( #introducing-new-modules )
21
+ - [ Deprecations] ( #deprecations )
22
+ - [ Involving the TSC] ( #involving-the-tsc )
12
23
* [ Landing Pull Requests] ( #landing-pull-requests )
13
- - [ Technical HOWTO] ( #technical-howto )
14
- - [ I Just Made a Mistake] ( #i-just-made-a-mistake )
15
- - [ Long Term Support] ( #long-term-support )
24
+ - [ Technical HOWTO] ( #technical-howto )
25
+ - [ Troubleshooting] ( #troubleshooting )
26
+ - [ I Just Made a Mistake] ( #i-just-made-a-mistake )
27
+ - [ Long Term Support] ( #long-term-support )
28
+ - [ What is LTS?] ( #what-is-lts )
29
+ - [ How does LTS work?] ( #how-does-lts-work )
30
+ - [ Landing semver-minor commits in LTS] ( #landing-semver-minor-commits-in-lts )
31
+ - [ How are LTS Branches Managed?] ( #how-are-lts-branches-managed )
32
+ - [ How can I help?] ( #how-can-i-help )
33
+ - [ How is an LTS release cut?] ( #how-is-an-lts-release-cut )
16
34
17
35
This document contains information for Collaborators of the Node.js
18
36
project regarding maintaining the code, documentation and issues.
@@ -24,53 +42,64 @@ understand the project governance model as outlined in
24
42
25
43
## Issues and Pull Requests
26
44
27
- Courtesy should ** always** be shown to individuals submitting issues and pull
28
- requests to the Node.js project. Be welcoming to first time contributors,
29
- identified by the GitHub ![ badge] ( ./doc/first_timer_badge.png ) badge.
45
+ ### Managing Issues and Pull Requests
30
46
31
47
Collaborators should feel free to take full responsibility for
32
48
managing issues and pull requests they feel qualified to handle, as
33
49
long as this is done while being mindful of these guidelines, the
34
- opinions of other Collaborators and guidance of the TSC.
50
+ opinions of other Collaborators and guidance of the [ TSC] [ ] . They
51
+ may also notify other qualified parties for more input on an issue
52
+ or a pull request.
53
+ [ See "Who to CC in issues"] ( ./doc/onboarding-extras.md#who-to-cc-in-issues )
54
+
55
+ ### Welcoming First-Time Contributiors
35
56
36
- Collaborators may ** close** any issue or pull request they believe is
57
+ Courtesy should always be shown to individuals submitting issues and pull
58
+ requests to the Node.js project. Be welcoming to first-time contributors,
59
+ identified by the GitHub ![ badge] ( ./doc/first_timer_badge.png ) badge.
60
+
61
+ For first-time contributors, check if the commit author is the same as the
62
+ pull request author, and ask if they have configured their git
63
+ username and email to their liking as per [ this guide] [ git-username ] .
64
+ This is to make sure they would be promoted to "contributor" once
65
+ their pull request gets landed.
66
+
67
+ ### Closing Issues and Pull Requests
68
+
69
+ Collaborators may close any issue or pull request they believe is
37
70
not relevant for the future of the Node.js project. Where this is
38
71
unclear, the issue should be left open for several days to allow for
39
72
additional discussion. Where this does not yield input from Node.js
40
73
Collaborators or additional evidence that the issue has relevance, the
41
74
issue may be closed. Remember that issues can always be re-opened if
42
75
necessary.
43
76
44
- [ ** See "Who to CC in issues"** ] ( ./doc/onboarding-extras.md#who-to-cc-in-issues )
45
-
46
77
## Accepting Modifications
47
78
48
79
All modifications to the Node.js code and documentation should be
49
80
performed via GitHub pull requests, including modifications by
50
- Collaborators and TSC members.
81
+ Collaborators and TSC members. A pull request must be reviewed, and usually
82
+ must also be tested with CI, before being landed into the codebase.
83
+
84
+ ### Code Reviews and Consensus Seeking
51
85
52
86
All pull requests must be reviewed and accepted by a Collaborator with
53
87
sufficient expertise who is able to take full responsibility for the
54
88
change. In the case of pull requests proposed by an existing
55
89
Collaborator, an additional Collaborator is required for sign-off.
56
90
57
91
In some cases, it may be necessary to summon a qualified Collaborator
58
- to a pull request for review by @-mention.
92
+ or a Github team to a pull request for review by @-mention.
93
+ [ See "Who to CC in issues"] ( ./doc/onboarding-extras.md#who-to-cc-in-issues )
59
94
60
95
If you are unsure about the modification and are not prepared to take
61
96
full responsibility for the change, defer to another Collaborator.
62
97
63
- Before landing pull requests, sufficient time should be left for input
64
- from other Collaborators. Leave at least 48 hours during the week and
65
- 72 hours over weekends to account for international time differences
66
- and work schedules. Trivial changes (e.g. those which fix minor bugs
67
- or improve performance without affecting API or causing other
68
- wide-reaching impact), and focused changes that affect only documentation
69
- and/or the test suite, may be landed after a shorter delay if they have
70
- multiple approvals.
71
-
72
- For first time contributors, ask the author if they have configured their git
73
- username and email to their liking as per [ this guide] [ git-username ] .
98
+ If any Collaborator objects to a change * without giving any additional
99
+ explanation or context* , and the objecting Collaborator fails to respond to
100
+ explicit requests for explanation or context within a reasonable period of
101
+ time, the objection may be dismissed. Note that this does not apply to
102
+ objections that are explained.
74
103
75
104
For non-breaking changes, if there is no disagreement amongst
76
105
Collaborators, a pull request may be landed given appropriate review.
@@ -80,12 +109,32 @@ elevate discussion to the TSC for resolution (see below).
80
109
81
110
Breaking changes (that is, pull requests that require an increase in
82
111
the major version number, known as ` semver-major ` changes) must be
83
- elevated for review by the TSC. This does not necessarily mean that the
84
- PR must be put onto the TSC meeting agenda. If multiple TSC members
85
- approve (` LGTM ` ) the PR and no Collaborators oppose the PR, it can be
86
- landed. Where there is disagreement among TSC members or objections
87
- from one or more Collaborators, ` semver-major ` pull requests should be
88
- put on the TSC meeting agenda.
112
+ [ elevated for review by the TSC] ( #involving-the-tsc ) .
113
+ This does not necessarily mean that the PR must be put onto the TSC meeting
114
+ agenda. If multiple TSC members approve (` LGTM ` ) the PR and no Collaborators
115
+ oppose the PR, it can be landed. Where there is disagreement among TSC members
116
+ or objections from one or more Collaborators, ` semver-major ` pull requests
117
+ should be put on the TSC meeting agenda.
118
+
119
+ ### Waiting for Approvals
120
+
121
+ Before landing pull requests, sufficient time should be left for input
122
+ from other Collaborators. In general, leave at least 48 hours during the
123
+ week and 72 hours over weekends to account for international time
124
+ differences and work schedules. However, certain types of pull requests
125
+ can be fast-tracked and may be landed after a shorter delay:
126
+
127
+ * Focused changes that affect only documentation and/or the test suite.
128
+ ` code-and-learn ` and ` good-first-issue ` pull requests typically fall
129
+ into this category.
130
+ * Changes that revert commit(s) and/or fix regressions.
131
+
132
+ When a pull request is deemed suitable to be fast-tracked, label it with
133
+ ` fast-track ` . The pull request can be landed once 2 or more collaborators
134
+ approve both the pull request and the fast-tracking request, and the necessary
135
+ CI testing is done.
136
+
137
+ ### Testing and CI
89
138
90
139
All bugfixes require a test case which demonstrates the defect. The
91
140
test should * fail* before the change, and * pass* after the change.
@@ -94,12 +143,6 @@ All pull requests that modify executable code should be subjected to
94
143
continuous integration tests on the
95
144
[ project CI server] ( https://ci.nodejs.org/ ) .
96
145
97
- If any Collaborator objects to a change * without giving any additional
98
- explanation or context* , and the objecting Collaborator fails to respond to
99
- explicit requests for explanation or context within a reasonable period of
100
- time, the objection may be dismissed. Note that this does not apply to
101
- objections that are explained.
102
-
103
146
#### Useful CI Jobs
104
147
105
148
* [ ` node-test-pull-request ` ] ( https://ci.nodejs.org/job/node-test-pull-request/ )
@@ -172,20 +215,8 @@ using an API in a manner currently undocumented achieves a particular useful
172
215
result, a decision will need to be made whether or not that falls within the
173
216
supported scope of that API; and if it does, it should be documented.
174
217
175
- Breaking changes to internal elements are permitted in semver-patch or
176
- semver-minor commits but Collaborators should take significant care when
177
- making and reviewing such changes. Before landing such commits, an effort
178
- must be made to determine the potential impact of the change in the ecosystem
179
- by analyzing current use and by validating such changes through ecosystem
180
- testing using the [ Canary in the Goldmine] ( https://github.com/nodejs/citgm )
181
- tool. If a change cannot be made without ecosystem breakage, then TSC review is
182
- required before landing the change as anything less than semver-major.
183
-
184
- If a determination is made that a particular internal API (for instance, an
185
- underscore ` _ ` prefixed property) is sufficiently relied upon by the ecosystem
186
- such that any changes may break user code, then serious consideration should be
187
- given to providing an alternative Public API for that functionality before any
188
- breaking changes are made.
218
+ See [ Breaking Changes to Internal Elements] ( #breaking-changes-to-internal-elements )
219
+ on how to handle those types of changes.
189
220
190
221
### Breaking Changes
191
222
@@ -200,6 +231,13 @@ changing error messages in any way, altering expected timing of an event (e.g.
200
231
moving from sync to async responses or vice versa), and changing the
201
232
non-internal side effects of using a particular API.
202
233
234
+ Purely additive changes (e.g. adding new events to ` EventEmitter `
235
+ implementations, adding new arguments to a method in a way that allows
236
+ existing code to continue working without modification, or adding new
237
+ properties to an options argument) are semver-minor changes.
238
+
239
+ #### Breaking Changes and Deprecations
240
+
203
241
With a few notable exceptions outlined below, when backwards incompatible
204
242
changes to a * Public* API are necessary, the existing API * must* be deprecated
205
243
* first* and the new API either introduced in parallel or added after the next
@@ -216,22 +254,36 @@ Exception to this rule is given in the following cases:
216
254
Such changes * must* be handled as semver-major changes but MAY be landed
217
255
without a [ Deprecation cycle] ( #deprecation-cycle ) .
218
256
219
- From time-to-time, in particularly exceptional cases, the TSC may be asked to
220
- consider and approve additional exceptions to this rule.
221
-
222
- Purely additive changes (e.g. adding new events to EventEmitter
223
- implementations, adding new arguments to a method in a way that allows
224
- existing code to continue working without modification, or adding new
225
- properties to an options argument) are handled as semver-minor changes.
226
-
227
257
Note that errors thrown, along with behaviors and APIs implemented by
228
258
dependencies of Node.js (e.g. those originating from V8) are generally not
229
259
under the control of Node.js and therefore * are not directly subject to this
230
260
policy* . However, care should still be taken when landing updates to
231
261
dependencies when it is known or expected that breaking changes to error
232
262
handling may have been made. Additional CI testing may be required.
233
263
234
- #### When breaking changes actually break things
264
+ From time-to-time, in particularly exceptional cases, the TSC may be asked to
265
+ consider and approve additional exceptions to this rule.
266
+
267
+ For more information, see [ Deprecations] ( #deprecations ) .
268
+
269
+ #### Breaking Changes to Internal Elements
270
+
271
+ Breaking changes to internal elements are permitted in semver-patch or
272
+ semver-minor commits but Collaborators should take significant care when
273
+ making and reviewing such changes. Before landing such commits, an effort
274
+ must be made to determine the potential impact of the change in the ecosystem
275
+ by analyzing current use and by validating such changes through ecosystem
276
+ testing using the [ Canary in the Goldmine] ( https://github.com/nodejs/citgm )
277
+ tool. If a change cannot be made without ecosystem breakage, then TSC review is
278
+ required before landing the change as anything less than semver-major.
279
+
280
+ If a determination is made that a particular internal API (for instance, an
281
+ underscore ` _ ` prefixed property) is sufficiently relied upon by the ecosystem
282
+ such that any changes may break user code, then serious consideration should be
283
+ given to providing an alternative Public API for that functionality before any
284
+ breaking changes are made.
285
+
286
+ #### When Breaking Changes Actually Break Things
235
287
236
288
Because breaking (semver-major) changes are permitted to land on the master
237
289
branch at any time, at least some subset of the user ecosystem may be adversely
@@ -349,12 +401,13 @@ Changes" section of the release notes.
349
401
350
402
### Involving the TSC
351
403
352
- Collaborators may opt to elevate pull requests or issues to the TSC for
353
- discussion by assigning the ` tsc-review ` label. This should be done
354
- where a pull request:
404
+ Collaborators may opt to elevate pull requests or issues to the [ TSC] [ ] for
405
+ discussion by assigning the ` tsc-review ` label or @-mentioning the
406
+ ` @nodejs/tsc ` Github team. This should be done where a pull request:
355
407
356
- - has a significant impact on the codebase,
357
- - is inherently controversial; or
408
+ - is labeled ` semver-major ` , or
409
+ - has a significant impact on the codebase, or
410
+ - is inherently controversial, or
358
411
- has failed to reach consensus amongst the Collaborators who are
359
412
actively participating in the discussion.
360
413
@@ -681,3 +734,4 @@ LTS working group and the Release team.
681
734
[ Enhancement Proposal ] : https://github.com/nodejs/node-eps
682
735
[ git-username ] : https://help.github.com/articles/setting-your-username-in-git/
683
736
[ `node-core-utils` ] : https://github.com/nodejs/node-core-utils
737
+ [ TSC ] : https://github.com/nodejs/TSC
0 commit comments