Lines Matching +full:pull +full:- +full:requests
5 * [Issues and pull requests](#issues-and-pull-requests)
6 * [Welcoming first-time contributors](#welcoming-first-time-contributors)
7 * [Closing issues and pull requests](#closing-issues-and-pull-requests)
8 * [Author ready pull requests](#author-ready-pull-requests)
9 * [Handling own pull requests](#handling-own-pull-requests)
10 * [Security issues](#managing-security-issues)
11 * [Accepting modifications](#accepting-modifications)
12 * [Code reviews](#code-reviews)
13 * [Consensus seeking](#consensus-seeking)
14 * [Waiting for approvals](#waiting-for-approvals)
15 * [Testing and CI](#testing-and-ci)
16 * [Useful Jenkins CI jobs](#useful-jenkins-ci-jobs)
17 * [Starting a Jenkins CI job](#starting-a-jenkins-ci-job)
18 * [Internal vs. public API](#internal-vs-public-api)
19 * [Breaking changes](#breaking-changes)
20 * [Breaking changes and deprecations](#breaking-changes-and-deprecations)
21 * [Breaking changes to internal elements](#breaking-changes-to-internal-elements)
22 * [Unintended breaking changes](#unintended-breaking-changes)
23 * [Reverting commits](#reverting-commits)
24 * [Introducing new modules](#introducing-new-modules)
25 * [Additions to Node-API](#additions-to-n-api)
27 * [Involving the TSC](#involving-the-tsc)
28 * [Landing pull requests](#landing-pull-requests)
29 * [Using `git-node`](#using-git-node)
30 * [Technical HOWTO](#technical-howto)
32 * [I made a mistake](#i-made-a-mistake)
33 * [Long Term Support](#long-term-support)
34 * [What is LTS?](#what-is-lts)
35 * [How are LTS branches managed?](#how-are-lts-branches-managed)
36 * [How can I help?](#how-can-i-help)
37 * [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker)
44 ## Issues and pull requests
47 [TSC][]. Notify other qualified parties for more input on an issue or a pull
48 request. See [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker).
50 ### Welcoming first-time contributors
52 Always show courtesy to individuals submitting issues and pull requests. Be
53 welcoming to first-time contributors, identified by the GitHub
54  badge.
56 For first-time contributors, check if the commit author is the same as the pull
57 request author. This way, once their pull request lands, GitHub will show them
59 [username][git-username] and [email][git-email] to their liking.
61 ### Closing issues and pull requests argument
63 Collaborators can close any issue or pull request that is not relevant to the
64 future of the Node.js project. Where this is unclear, leave the issue or pull
66 evidence that the issue or pull request has relevance, close it. Remember that
67 issues and pull requests can always be re-opened if necessary.
69 ### Author ready pull requests argument
71 A pull request is _author ready_ when:
77 Please always add the `author ready` label to the pull request in that case.
80 ### Handling own pull requests argument
82 When you open a pull request, [start a CI](#testing-and-ci) right away. Later,
85 As soon as the pull request is ready to land, please do so. This allows other
86 collaborators to focus on other pull requests. If your pull request is not ready
87 to land but is [author ready](#author-ready-pull-requests), add the
88 `author ready` label. If you wish to land the pull request yourself, use the
89 "assign yourself" link to self-assign it.
99 [premature-disclosures](https://github.com/nodejs/premature-disclosures).
100 * For any related pull requests, create an associated issue in the
101 `premature-disclosures` repository. Add a copy of the patch for the
102 pull request to the issue. Add screenshots of discussion from the pull request
105 pull request using Node.js (team) as the account organization.
106 * Open a new issue in the public repository with the title `FYI - pull request
108 > FYI @xxxx we asked GitHub to delete your pull request while we work on
111 `premature-disclosures` repository.
115 Contributors propose modifications to Node.js using GitHub pull requests. This
116 includes modifications proposed by TSC members and other collaborators. A pull
121 At least two collaborators must approve a pull request before the pull request
122 lands. One collaborator approval is enough if the pull request has been open
125 Approving a pull request indicates that the collaborator accepts responsibility
130 In some cases, it might be necessary to summon a GitHub team to a pull request
131 for review by @-mention.
132 See [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker).
134 If you are the first collaborator to approve a pull request that has no CI yet,
135 please [start one](#testing-and-ci). Please also start a new CI if the
136 pull request creator pushed new code since the last CI run.
140 A pull request can land if it has the needed [approvals](#code-reviews),
141 [CI](#testing-and-ci), [wait time](#waiting-for-approvals) and no
142 [outstanding objections](#objections). [Breaking changes](#breaking-changes)
143 must receive [TSC review](#involving-the-tsc) in addition to other
144 requirements. If a pull request meets all requirements except the
145 [wait time](#waiting-for-approvals), please add the
146 [`author ready`](#author-ready-pull-requests) label.
150 Collaborators can object to a pull request by using the "Request
152 objection. Any pull request objection must include a clear reason for that
154 towards consensus about the direction of the pull request. Where possible,
163 Pull requests with outstanding objections must remain open until all
166 adding the `tsc-agenda` label to the issue.
170 * [How to Do Code Reviews Like a Human (Part One)](https://mtlynch.io/human-code-reviews-1/)
171 * [How to Do Code Reviews Like a Human (Part Two)](https://mtlynch.io/human-code-reviews-2/)
172 * [Code Review Etiquette](https://css-tricks.com/code-review-etiquette/)
176 Before landing pull requests, allow 48 hours for input from other collaborators.
177 Certain types of pull requests can be fast-tracked and can land after a shorter
181 * `code-and-learn` tasks often fall into this category.
182 * `good-first-issue` pull requests might also be suitable.
187 To propose fast-tracking a pull request, apply the `fast-track` label. Then add
190 If someone disagrees with the fast-tracking request, remove the label. Do not
191 fast-track the pull request in that case.
193 The pull request can be fast-tracked if two collaborators approve the
194 fast-tracking request. To land, the pull request itself still needs two
197 Collaborators can request fast-tracking of pull requests they did not author.
198 In that case only, the request itself is also one fast-track approval. Upvote
206 Do not land any pull requests without the necessary passing CI runs.
208 yellow) [Jenkins CI](https://ci.nodejs.org/) is also required if the pull
215 Changes in the following folders (except comment-only changes) are guaranteed to
225 * `tools/inspector-protocol/`
234 * `tools/build-addons.js`
244 If there are GitHub Actions CI failures unrelated to the change in the pull
245 request, try "Re-run all jobs". It's under the " Re-run jobs" button, on the
246 right-hand side of "Checks" tab.
248 If there are Jenkins CI failures unrelated to the change in the pull request,
250 `node-test-pull-request` job. It will preserve all the green results from the
251 current job but re-run everything else. Start a fresh CI if more than seven days
257 * [`node-test-pull-request`](https://ci.nodejs.org/job/node-test-pull-request/)
258 is the CI job to test pull requests. It runs the `build-ci` and `test-ci`
261 * [`citgm-smoker`](https://ci.nodejs.org/job/citgm-smoker/)
266 * [`node-stress-single-test`](https://ci.nodejs.org/job/node-stress-single-test/)
270 * [`node-test-commit-v8-linux`](https://ci.nodejs.org/job/node-test-commit-v8-linux/)
274 * [`node-test-commit-custom-suites-freestyle`](https://ci.nodejs.org/job/node-test-commit-custom-su…
278 used in other CI test runs (such as `--worker`).
289 (e.g. for `master` -> `refs/heads/master`).
290 For pull requests, it will look like `refs/pull/PR_NUMBER/head`
291 (e.g. for pull request #42 -> `refs/pull/42/head`).
292 * `REBASE_ONTO`: Change that to `origin/master` so the pull request gets rebased
293 onto master. This can especially be important for pull requests that have been
301 Copy/paste the URL for the job into a comment in the pull request.
302 [`node-test-pull-request`](https://ci.nodejs.org/job/node-test-pull-request/)
305 The [`node-test-pull-request`](https://ci.nodejs.org/job/node-test-pull-request/)
306 CI job can be started by adding the `request-ci` label into the pull request.
307 Once this label is added, `github-actions bot` will start
308 the `node-test-pull-request` automatically. If the `github-actions bot`
309 is unable to start the job, it will update the label with `request-ci-failed`.
322 public, though, if it is re-exported by code in `lib/*.js`.
324 Non-exported `Symbol` properties and methods are internal.
334 For undocumented APIs that are public, open a pull request documenting the API.
338 At least two TSC members must approve backward-incompatible changes to the
352 Existing stable public APIs that change in a backward-incompatible way must
357 * Altering the timing and non-internal side effects of the public API.
359 * One-time exceptions granted by the TSC.
365 Breaking changes to internal elements can occur in semver-patch or semver-minor
369 If a change will cause ecosystem breakage, then it is semver-major. Consider
374 Sometimes, a change intended to be non-breaking turns out to be a breaking
376 it. As an alternative to reverting, the TSC can apply the semver-major label
377 after-the-fact.
384 metadata. Raise a pull request like any other change.
395 soon as possible. Link to the pull request that introduces the new core module
398 For pull requests introducing new core modules:
401 * Land only after sign-off from at least two TSC members.
403 Experimental until a semver-major release.
405 ### Additions to Node-API
407 Node-API provides an ABI-stable API guaranteed for future Node.js versions.
408 Node-API additions call for unusual care and scrutiny. If a change adds to
410 consult [the relevant guide](https://github.com/nodejs/node/blob/HEAD/doc/guides/adding-new-napi-ap…
417 * Documentation-Only Deprecation
422 * Might cause a runtime warning with the [`--pending-deprecation`][] flag or
427 * If used with the [`--throw-deprecation`][] flag, will throw a runtime error.
429 * End-of-Life
431 * Backward-incompatible changes including complete removal of such APIs can
434 Apply the `notable change` label to all pull requests that introduce
435 Documentation-Only Deprecations. Such deprecations have no impact on code
436 execution. Thus, they are not breaking changes (`semver-major`).
438 Runtime Deprecations and End-of-Life APIs (internal or public) are breaking
439 changes (`semver-major`). The TSC can make exceptions, deciding that one of
442 Avoid Runtime Deprecations when an alias or a stub/no-op will suffice. An alias
449 example, due to removal of an End-of-Life deprecated API).
451 <a id="deprecation-cycle"></a>
453 the three Deprecation levels. Documentation-Only Deprecations can land in a
457 No API can change to End-of-Life without going through a Runtime Deprecation
458 cycle. There is no rule that deprecated code must progress to End-of-Life.
459 Documentation-Only and Runtime Deprecations can remain in place for an unlimited
463 as soon as possible. If possible, do it before the pull request adding the
466 Use the `notable-change` label on pull requests that add or change the
471 Collaborators can opt to elevate pull requests or issues to the [TSC][].
472 Do this if a pull request or issue:
474 * Is labeled `semver-major`, or
479 @-mention the `@nodejs/tsc` GitHub team if you want to elevate an issue to the
480 [TSC][]. Do not use the GitHub UI on the right-hand side to assign to
485 ## Landing pull requests
487 1. Avoid landing pull requests that have someone else as an assignee. Authors
488 who wish to land their own pull requests will self-assign them. Sometimes, an
491 1. Never use GitHub's green ["Merge pull request"][] button. Reasons for not
494 * The "Squash and merge" method will add metadata (the pull request #) to the
495 commit title. If more than one author contributes to the pull request,
502 issues. Run a new CI any time someone pushes new code to the pull request.
506 as a reference. See [this commit][commit-example] as an example.
508 For pull requests from first-time contributors, be
509 [welcoming](#welcoming-first-time-contributors). Also, verify that their git
512 All commits should be self-contained, meaning every commit should pass all
515 ### Using `git-node`
517 In most cases, using [the `git-node` command][git-node] of [`node-core-utils`][]
518 is enough to land a pull request. If you discover a problem when using
519 this tool, please file an issue [to the issue tracker][node-core-utils-issues].
524 $ npm install -g node-core-utils
528 To use `node-core-utils`, you will need a GitHub access token. If you do not
529 have one, `node-core-utils` will create one for you the first time you use it.
530 To do this, it will ask for your GitHub password and two-factor authentication
532 [the `node-core-utils` guide][node-core-utils-credentials].
537 pull request rather than rely on `git-node`.
545 $ git am --abort
546 $ git rebase --abort
556 [CONTRIBUTING.md](./contributing/pull-requests.md#step-1-fork)):
560 $ git merge --ff-only upstream/master
566 $ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am --whitespace=fix
569 If the merge fails even though recent CI runs were successful, try a 3-way
573 $ git am --abort
574 $ curl -L https://github.com/nodejs/node/pull/xxx.patch | git am -3 --whitespace=fix
577 If the 3-way merge succeeds, check the results against the original pull
580 If the 3-way merge fails, then it is most likely that a conflicting pull request
583 Check and re-review the changes:
598 $ git rebase -i upstream/master
619 # These lines can be re-ordered; they are executed from top to bottom.
652 [`git node metadata`][git-node-metadata] command can generate the metadata
655 * Required: A `PR-URL:` line that references the full GitHub URL of the pull
662 * Required: A `Reviewed-By: Name <email>` line for each collaborator who
665 pull request.
669 precaution, run tests (`make -j4 test` or `vcbuild test`).
672 [core-validate-commit](https://github.com/nodejs/core-validate-commit).
675 $ git rev-list upstream/master...HEAD | xargs core-validate-commit
678 Optional: For your own commits, force push the amended commit to the pull
680 --force-with-lease origin master:bugfix`. Don't close the pull request.
682 status rather than the red closed status. If you close the pull request
683 before GitHub adjusts its status, it will show up as a 0 commit pull
694 Close the pull request with a "Landed in `<commit hash>`" comment. Even if
695 your pull request shows the purple merged status,
708 ! [rejected] master -> master (fetch first)
712 hint: 'git pull ...') before pushing again.
713 hint: See the 'Note about fast-forwards' in 'git push --help' for details.
717 To fix this, pull with rebase from upstream, run the tests again, and (if the
721 git pull upstream master --rebase
722 make -j4 test
730 (`git push -f`). This is generally forbidden as it creates conflicts in other
731 people's forks. It is permissible for simpler slip-ups such as typos in commit
734 10-minute period passes, consider the commit final.
735 * Use `--force-with-lease` to reduce the chance of overwriting someone else's
742 Long Term Support (LTS) guarantees 30-month support cycles for specific Node.js
744 [in the full release plan](https://github.com/nodejs/Release#release-plan). Once
751 corresponding staging branch (v10.x-staging, v8.x-staging, etc.).
753 Commits that land on master are cherry-picked to each staging branch as
754 appropriate. If a change applies only to the LTS branch, open the pull request
764 When you send your pull request, please state if your change is breaking. Also
768 There are several LTS-related labels:
770 * `lts-watch-` labels are for pull requests to consider for landing in staging
771 branches. For example, `lts-watch-v10.x` would be for a change
772 to consider for the `v10.x-staging` branch.
774 * `land-on-` are for pull requests that should land in a future v*.x
775 release. For example, `land-on-v10.x` would be for a change to land in Node.js
778 Any collaborator can attach these labels to any pull request/issue. As commits
779 land on the staging branches, the backporter removes the `lts-watch-` label.
780 Likewise, as commits land in an LTS release, the releaser removes the `land-on-`
783 Attach the appropriate `lts-watch-` label to any pull request that
789 | --- | --- …
802 | `lib/inspector.js`, `src/inspector_*` | @nodejs/v8-inspector …
812 | `src/node_api.*` | @nodejs/n-api …
820 | platform specific | @nodejs/platform-{aix,arm,freebsd,macos,ppc,smartos,s3…
822 | upgrading c-ares | @rvagg …
823 | upgrading http-parser | @nodejs/http, @nodejs/http2 …
826 | upgrading V8 | @nodejs/V8, @nodejs/post-mortem …
827 | Embedded use or delivery of Node.js | @nodejs/delivery-channels …
829 When things need extra attention, are controversial, or `semver-major`:
832 If you cannot find who to cc for a file, `git shortlog -n -s <file>` can help.
838 * `confirmed-bug`: Bugs you have verified
840 * `feature request`: Any issue that requests a new feature
843 * `tsc-agenda`: Open issues and pull requests with this label will be added to
846 ---
848 * `author-ready` - A pull request is _author ready_ when:
851 semver-major pull requests).
854 Please always add the `author ready` label to pull requests that qualify.
858 ---
860 * `semver-{minor,major}`
862 something, go for semver-major
873 * `dont-land-on-v?.x`
876 * `land-on-v?.x`
877 * Used by releasers to mark a pull request as scheduled for inclusion in an
879 * Applied to the original pull request for clean cherry-picks, to the backport
880 pull request otherwise
881 * `backport-requested-v?.x`
882 * Used to indicate that a pull request needs a manual backport to a branch in
884 * Typically applied by a releaser when the pull request does not apply cleanly
886 * Will be replaced by either `dont-land-on-v?.x` or `backported-to-v?.x`
887 * `backported-to-v?.x`
888 * Applied to pull requests for which a backport pull request has been merged
889 * `lts-watch-v?.x`
890 * Applied to pull requests which the Release working group should consider
893 effective as messaging to non-collaborators
894 * `release-agenda`
896 * (for example semver-minor changes that need or should go into an LTS
900 `v?.x-staging` branch
914 ["Merge pull request"]: https://help.github.com/articles/merging-a-pull-request/#merging-a-pull-req…
917 [Stability Index]: ../api/documentation.md#stability-index
919 [`--pending-deprecation`]: ../api/cli.md#--pending-deprecation
920 [`--throw-deprecation`]: ../api/cli.md#--throw-deprecation
921 [`node-core-utils`]: https://github.com/nodejs/node-core-utils
922 [backporting guide]: backporting-to-release-lines.md
923 [commit message guidelines]: contributing/pull-requests.md#commit-message-guidelines
924 [commit-example]: https://github.com/nodejs/node/commit/b636ba8186
925 [git-email]: https://help.github.com/articles/setting-your-commit-email-address-in-git/
926 [git-node]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md
927 [git-node-metadata]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-…
928 [git-username]: https://help.github.com/articles/setting-your-username-in-git/
929 [node-core-utils-credentials]: https://github.com/nodejs/node-core-utils#setting-up-credentials
930 [node-core-utils-issues]: https://github.com/nodejs/node-core-utils/issues