Lines Matching +full:pull +full:- +full:requests
1 # Pull requests
4 * [Setting up your local environment](#setting-up-your-local-environment)
5 * [Step 1: Fork](#step-1-fork)
6 * [Step 2: Branch](#step-2-branch)
7 * [The process of making changes](#the-process-of-making-changes)
8 * [Step 3: Code](#step-3-code)
9 * [Step 4: Commit](#step-4-commit)
10 * [Commit message guidelines](#commit-message-guidelines)
11 * [Step 5: Rebase](#step-5-rebase)
12 * [Step 6: Test](#step-6-test)
13 * [Step 7: Push](#step-7-push)
14 * [Step 8: Opening the pull request](#step-8-opening-the-pull-request)
15 * [Step 9: Discuss and update](#step-9-discuss-and-update)
16 * [Approval and request changes workflow](#approval-and-request-changes-workflow)
17 * [Step 10: Landing](#step-10-landing)
18 * [Reviewing pull requests](#reviewing-pull-requests)
19 * [Review a bit at a time](#review-a-bit-at-a-time)
20 * [Be aware of the person behind the code](#be-aware-of-the-person-behind-the-code)
21 * [Respect the minimum wait time for comments](#respect-the-minimum-wait-time-for-comments)
22 * [Abandoned or stalled pull requests](#abandoned-or-stalled-pull-requests)
23 * [Approving a change](#approving-a-change)
24 … opinions about what belongs in Node.js](#accept-that-there-are-different-opinions-about-what-belo…
25 * [Performance is not everything](#performance-is-not-everything)
26 * [Continuous integration testing](#continuous-integration-testing)
28 * [Commit squashing](#commit-squashing)
29 * [Getting approvals for your pull request](#getting-approvals-for-your-pull-request)
30 * [Waiting until the pull request gets landed](#waiting-until-the-pull-request-gets-landed)
31 * [Check out the collaborator guide](#check-out-the-collaborator-guide)
32 * [Appendix: subsystems](#appendix-subsystems)
43 [project collaborators](https://github.com/nodejs/node/#current-project-team-members).
46 [OpenJS Foundation Slack](https://slack-invite.openjsf.org/). Interesting
48 [#nodejs](https://openjs-foundation.slack.com/archives/CK9Q4MB53) for general
50 [#nodejs-core](https://openjs-foundation.slack.com/archives/C019Y2T6STH) for
63 [IDE configs](https://github.com/nodejs/node-code-ide-configs).
103 git checkout -b my-branch -t upstream/HEAD
110 Pull requests in Node.js typically involve changes to
131 <!-- YAML
133 -->
139 [C++ Style Guide](cpp-style-guide.md), as well as the
147 commits any single pull request may have, and many contributors find it easier
156 notes about [commit squashing](#commit-squashing).
168 * be prefixed with the name of the changed [subsystem](#appendix-subsystems)
169 and start with an imperative verb. Check the output of `git log --oneline
188 * `Refs: https://eslint.org/docs/rules/space-in-parens.html`
189 * `Refs: https://github.com/nodejs/node/pull/3615`
191 5. If your commit introduces a breaking change (`semver-major`), it should
201 things in more detail. Please word-wrap to keep columns to 72 characters or
205 Refs: https://eslint.org/docs/rules/space-in-parens.html
211 contributor landing the pull request will ensure that everything follows
239 Before submitting your changes in a pull request, always run the full Node.js
243 ./configure && make -j4 test
246 We can speed up the builds by using [Ninja](https://ninja-build.org/). For more
248 [Building Node.js with Ninja](building-node-with-ninja.md).
263 begin the process of opening a pull request by pushing your working branch to
267 git push origin my-branch
270 ### Step 8: Opening the pull request
272 From within GitHub, opening a new pull request will present you with a
273 [pull request template][]. Please try to do your best at filling out the
276 Once opened, pull requests are usually reviewed within a few days.
285 You will probably get feedback or requests for changes to your pull request.
287 contributors may sign off on the pull request right away, others may have
291 To make changes to an existing pull request, make the changes to your local
293 GitHub will automatically update the pull request.
298 git push origin my-branch
307 git push --force-with-lease origin my-branch
310 **Important:** The `git push --force-with-lease` command is one of the few ways
314 the pull request.
319 Feel free to post a comment in the pull request to ping reviewers if you are
326 All pull requests require "sign off" in order to land. Whenever a contributor
327 reviews a pull request they may find specific details that they would like to
329 substantive changes to the code you have written. While such requests are
331 requests to change things that do not include concrete suggestions on _how_ to
337 short amount of time to review and are not ill-intended. Such issues can often
344 In order to land, a pull request needs to be reviewed and [approved][] by
346 pull request has been open for more than 7 days) and pass a
348 objections from other contributors, the pull request can be merged. If you find
349 your pull request waiting longer than you expect, see the
350 [notes about the waiting time](#waiting-until-the-pull-request-gets-landed).
352 When a collaborator lands your pull request, they will post
353 a comment to the pull request page mentioning the commit(s) it
354 landed as. GitHub might show the pull request as `Closed` at this
356 pull request against, you should see a commit with your name on it.
359 ## Reviewing pull requests
361 All Node.js contributors who choose to review and provide feedback on Pull
362 Requests have a responsibility to both the project and the individual making the
365 expect to be able to block a pull request from advancing simply because you say
367 to working with the contributor to make the pull request better.
372 When reviewing a pull request, the primary goals are for the codebase to improve
373 and for the person submitting the request to succeed. Even if a pull request
375 their effort was not wasted or unappreciated. Every pull request from a new
382 It is tempting to micro-optimize and make everything about relative performance,
399 Nits (requests for small changes that are not essential) are fine, but try to
400 avoid stalling the pull request. Most nits can typically be fixed by the
401 Node.js collaborator landing the pull request but they can also be an
408 commits or if they proved to be mistaken, please, [hide them][hiding-a-comment]
413 Be aware that _how_ you communicate requests and reviews in your feedback can
414 have a significant impact on the success of the pull request. Yes, we may land
421 There is a minimum waiting time which we try to respect for non-trivial
425 For non-trivial changes, pull requests must be left open for at least 48 hours.
427 from subject-matter experts. When in doubt, do not rush.
432 ### Abandoned or stalled pull requests argument
434 If a pull request appears to be abandoned or stalled, it is polite to first
439 address) in the commit log, or by using an `Author:` meta-data tag in the
446 work. Collaborators are not permitted to approve their own pull requests.
449 a pull request either by using GitHub's Approval Workflow, which is preferred,
457 Most importantly, after leaving such requests, it is courteous to make yourself
463 Change requests that are vague, dismissive, or unconstructive may also be
464 dismissed if requests for greater clarification go unanswered within a
467 Use `Changes requested` to block a pull request from landing. When doing so,
468 explain why you believe the pull request should not land along with an
493 accepted. Claims that a particular pull request will make things faster will
494 almost always be met by requests for performance [benchmark results][] that
501 If a particular pull request introduces a performance or functional
502 regression, rather than simply rejecting the pull request, take the time to
504 advice on what would make the pull request acceptable, and do not assume that
510 All pull requests that contain changes to code must be run through
516 test run for you as approvals for the pull request come in.
522 to fail on specific platforms or for so-called "flaky" tests to fail ("be red").
524 determine whether the failure was caused by the changes in the pull request.
530 In most cases, do not squash commits that you add to your pull request during
531 the review process. When the commits in your pull request land, they may be
533 commit message (including links to the pull request, links to relevant issues,
534 and the names of the reviewers). The commit history of your pull request,
535 however, will stay intact on the pull request page.
543 ### Getting approvals for your pull request
545 A pull request is approved either by saying LGTM, which stands for
547 GitHub's pull request review feature can be used during the process.
550 or [the official documentation](https://help.github.com/articles/reviewing-changes-in-pull-requests…
556 ### Waiting until the pull request gets landed
558 A pull request needs to stay open for at least 48 hours from when it is
561 collaborators may decide it doesn't need to wait. A pull request may well take
580 [nodejs/core-validate-commit][] repository.
581 More than one subsystem may be valid for any particular issue or pull request.
584 [CI (Continuous Integration) test run]: #continuous-integration-testing
587 [approved]: #getting-approvals-for-your-pull-request
588 [benchmark results]: writing-and-running-benchmarks.md
589 [collaborator guide]: collaborator-guide.md
590 [guide for writing tests in Node.js]: writing-tests.md
591 [hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment
593 [nodejs/core-validate-commit]: https://github.com/nodejs/core-validate-commit/blob/main/lib/rules/s…
594 [pull request template]: https://raw.githubusercontent.com/nodejs/node/HEAD/.github/PULL_REQUEST_TE…
595 [running tests]: ../../BUILDING.md#running-tests