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 * [CI testing](#ci-testing)
31 * [Waiting until the pull request gets landed](#waiting-until-the-pull-request-gets-landed)
32 * [Check out the collaborator guide](#check-out-the-collaborator-guide)
33 * [Appendix: subsystems](#appendix-subsystems)
44 [project collaborators](https://github.com/nodejs/node/#current-project-team-members).
47 [OpenJS Foundation Slack](https://slack-invite.openjsf.org/). Interesting
49 [#nodejs](https://openjs-foundation.slack.com/archives/CK9Q4MB53) for general
51 [#nodejs-dev](https://openjs-foundation.slack.com/archives/C019Y2T6STH) for
64 [IDE configs](https://github.com/nodejs/node-code-ide-configs).
104 $ git checkout -b my-branch -t upstream/master
111 The vast majority of pull requests opened against the `nodejs/node`
124 should follow the [Style Guide](../doc-style-guide.md). Code samples
132 <!-- YAML
134 -->
140 [C++ Style Guide](../cpp-style-guide.md), as well as the
148 commits any single pull request may have, and many contributors find it easier
157 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
185 * `Refs: https://eslint.org/docs/rules/space-in-parens.html`
186 * `Refs: https://github.com/nodejs/node/pull/3615`
188 5. If your commit introduces a breaking change (`semver-major`), it should
198 things in more detail. Please word-wrap to keep columns to 72 characters or
202 Refs: https://eslint.org/docs/rules/space-in-parens.html
208 contributor landing the pull request will ensure that everything follows
237 Before submitting your changes in a pull request, always run the full Node.js
241 $ ./configure && make -j4 test
255 begin the process of opening a pull request by pushing your working branch to
259 $ git push origin my-branch
262 ### Step 8: Opening the pull request
264 From within GitHub, opening a new pull request will present you with a
265 [pull request template][]. Please try to do your best at filling out the
268 Once opened, pull requests are usually reviewed within a few days.
272 You will probably get feedback or requests for changes to your pull request.
274 contributors may sign off on the pull request right away, others may have
278 To make changes to an existing pull request, make the changes to your local
280 GitHub will automatically update the pull request.
285 $ git push origin my-branch
288 It is also frequently necessary to synchronize your pull request with other
292 $ git fetch --all
294 $ git push --force-with-lease origin my-branch
297 **Important:** The `git push --force-with-lease` command is one of the few ways
299 risks. If in doubt, you can always ask for guidance in the pull request.
306 $ git commit --amend
307 $ git push --force-with-lease origin my-branch
313 Feel free to post a comment in the pull request to ping reviewers if you are
320 All pull requests require "sign off" in order to land. Whenever a contributor
321 reviews a pull request they may find specific details that they would like to
323 substantive changes to the code you have written. While such requests are
325 requests to change things that do not include concrete suggestions on *how* to
331 short amount of time to review and are not ill-intended. Such issues can often
338 In order to land, a pull request needs to be reviewed and [approved][] by
340 pull request has been open for more than 7 days) and pass a
342 objections from other contributors, the pull request can be merged. If you find
343 your pull request waiting longer than you expect, see the
344 [notes about the waiting time](#waiting-until-the-pull-request-gets-landed).
346 When a collaborator lands your pull request, they will post
347 a comment to the pull request page mentioning the commit(s) it
348 landed as. GitHub often shows the pull request as `Closed` at this
350 pull request against (probably `master`), you should see a commit with
353 ## Reviewing pull requests
355 All Node.js contributors who choose to review and provide feedback on Pull
356 Requests have a responsibility to both the project and the individual making the
359 expect to be able to block a pull request from advancing simply because you say
361 to working with the contributor to make the pull request better.
366 When reviewing a pull request, the primary goals are for the codebase to improve
367 and for the person submitting the request to succeed. Even if a pull request
369 their effort was not wasted or unappreciated. Every pull request from a new
376 It is tempting to micro-optimize and make everything about relative performance,
393 Nits (requests for small changes that are not essential) are fine, but try to
394 avoid stalling the pull request. Most nits can typically be fixed by the
395 Node.js collaborator landing the pull request but they can also be an
402 commits or if they proved to be mistaken, please, [hide them][hiding-a-comment]
407 Be aware that *how* you communicate requests and reviews in your feedback can
408 have a significant impact on the success of the pull request. Yes, we may land
415 There is a minimum waiting time which we try to respect for non-trivial
419 For non-trivial changes, pull requests must be left open for at least 48 hours.
421 from subject-matter experts. When in doubt, do not rush.
426 ### Abandoned or stalled pull requests argument
428 If a pull request appears to be abandoned or stalled, it is polite to first
433 address in the commit log, or by using an `Author:` meta-data tag in the
440 work. Collaborators are not permitted to approve their own pull requests.
443 a pull request either by using GitHub's Approval Workflow, which is preferred,
451 Most importantly, after leaving such requests, it is courteous to make yourself
457 Change requests that are vague, dismissive, or unconstructive may also be
458 dismissed if requests for greater clarification go unanswered within a
461 Use `Changes requested` to block a pull request from landing. When doing so,
462 explain why you believe the pull request should not land along with an
487 accepted. Claims that a particular pull request will make things faster will
488 almost always be met by requests for performance [benchmark results][] that
495 If a particular pull request introduces a performance or functional
496 regression, rather than simply rejecting the pull request, take the time to
498 advice on what would make the pull request acceptable, and do not assume that
504 All pull requests that contain changes to code must be run through
515 specific platforms or for so-called "flaky" tests to fail ("be red"). It is
517 whether the failure was caused by the changes in the pull request.
523 In most cases, do not squash commits that you add to your pull request during
524 the review process. When the commits in your pull request land, they may be
526 commit message (including links to the pull request, links to relevant issues,
527 and the names of the reviewers). The commit history of your pull request,
528 however, will stay intact on the pull request page.
536 ### Getting approvals for your pull request
538 A pull request is approved either by saying LGTM, which stands for
540 GitHub's pull request review feature can be used during the process.
543 or [the official documentation](https://help.github.com/articles/reviewing-changes-in-pull-requests…
551 Every pull request needs to be tested
556 for you as approvals for the pull request come in.
559 ### Waiting until the pull request gets landed
561 A pull request needs to stay open for at least 48 hours from when it is
564 collaborators may decide it doesn't need to wait. A pull request may well take
582 More than one subsystem may be valid for any particular issue or pull request.
585 [CI (Continuous Integration) test run]: #ci-testing
588 [approved]: #getting-approvals-for-your-pull-request
589 [benchmark results]: ../writing-and-running-benchmarks.md
590 [collaborator guide]: ../collaborator-guide.md
591 [guide for writing tests in Node.js]: ../writing-tests.md
592 [hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment
594 [pull request template]: https://raw.githubusercontent.com/nodejs/node/HEAD/.github/PULL_REQUEST_TE…
595 [running tests]: ../../../BUILDING.md#running-tests