Skip to main content

Overview

Pull Requests are the way concrete changes are made to the code, documentation, dependencies, and tools contained in the nodejs/node repository.

Dependencies

Node.js has several bundled dependencies in the deps/ and tools/ directories that are not part of the project proper. Changes to files in those directories should be sent to their respective projects.
Do not send patches for bundled dependencies directly to Node.js. We cannot accept such patches.
For more information, see the maintaining dependencies document.

Getting Help

If you’re unsure about dependencies or need help:

Setting Up Your Local Environment

Prerequisites

To get started, you will need:
  • git installed locally
  • Build dependencies for your operating system (see Building guide)
  • Optional: IDE-specific settings from IDE configs

Step 1: Fork

Fork the project on GitHub and clone your fork locally.
git clone [email protected]:username/node.git
cd node
git remote add upstream https://github.com/nodejs/node.git
git fetch upstream
Configure git so that it knows who you are:
git config user.name "J. Random User"
git config user.email "[email protected]"
If you would like for the GitHub UI to link the commit to your account and award you the Contributor label after the changes have been merged, make sure this local email is also added to your GitHub email list.

Step 2: Branch

As a best practice to keep your development environment as organized as possible, create local branches to work within. These should be created directly off of the upstream default branch.
git checkout -b my-branch -t upstream/HEAD

The Process of Making Changes

Step 3: Code

Pull requests in Node.js typically involve changes to one or more of:
  • C/C++ code in the src directory
  • JavaScript code in the lib directory
  • Documentation in doc/api
  • Tests in the test directory

Code Quality

If you are modifying code, run the linter:
# Unix / macOS
make lint

# Windows
vcbuild.bat lint

Documentation Style

Any documentation you write (including code comments and API documentation) should follow the Style Guide. When adding to or deprecating an API, use REPLACEME for the version number in the documentation YAML:
### `request.method`
<!-- YAML
added: REPLACEME
-->

* {string} The request method.

C++ Code

For contributing C++ code, review:

Step 4: Commit

It is a best practice to keep your changes as logically grouped as possible within individual commits. There is no limit to the number of commits any single pull request may have.
git add my/changed/files
git commit
Multiple commits often get squashed when they are landed. See the notes about commit squashing.

Commit Message Guidelines

A good commit message should describe what changed and why. First line should:
  • Contain a short description (preferably 50 characters or less, no more than 72 characters)
  • Be entirely in lowercase except for proper nouns, acronyms, and code references
  • Be prefixed with the subsystem name and start with an imperative verb
Examples:
net: add localAddress and localPort to Socket
src: fix typos in async_wrap.h
Second line:
  • Keep blank
Remaining lines:
  • Wrap at 72 columns (except for long URLs)
  • Provide detailed explanation of changes
References: If your patch fixes an open issue, add a reference at the end:
Fixes: https://github.com/nodejs/node/issues/1337
Refs: https://eslint.org/docs/rules/space-in-parens.html
Refs: https://github.com/nodejs/node/pull/3615
Breaking changes: If your commit introduces a breaking change (semver-major), include:
  • Reason for the breaking change
  • Situation that triggers it
  • Exact nature of the change
Sample complete commit message:
subsystem: explain the commit in one line

The body of the commit message should be one or more paragraphs, explaining
things in more detail. Please word-wrap to keep columns to 72 characters or
less.

Fixes: https://github.com/nodejs/node/issues/1337
Refs: https://eslint.org/docs/rules/space-in-parens.html
If you are new to contributing to Node.js, please try to do your best at conforming to these guidelines, but do not worry if you get something wrong. One of the existing contributors will help get things situated.

Step 5: Rebase

As a best practice, once you have committed your changes, use git rebase (not git merge) to synchronize your work with the main repository.
git fetch upstream HEAD
git rebase FETCH_HEAD
This ensures that your working branch has the latest changes from nodejs/node.

Step 6: Test

Bug fixes and features should always come with tests. A guide for writing tests in Node.js has been provided to make the process easier.
Before submitting your changes in a pull request, always run the full Node.js test suite.
Run tests on Unix / macOS:
./configure && make -j4 test
Run tests on Windows:
vcbuild test
For some configurations, running all tests might take a long time (an hour or more). To run a subset of the test suite, see the running tests section of the Building guide.

Step 7: Push

Once you are sure your commits are ready to go, with passing tests and linting, push your working branch to your fork on GitHub.
git push origin my-branch

Step 8: Opening the Pull Request

From within GitHub, opening a new pull request will present you with a pull request template. Please try to do your best at filling out the details.
To get feedback on your proposed change even though it is not ready to land, use the “Convert to draft” option in the GitHub UI. Do not use the wip label as it might not prevent the PR from landing before you are ready.
Once opened, pull requests are usually reviewed within a few days.

Step 9: Discuss and Update

You will probably get feedback or requests for changes to your pull request. This is a big part of the submission process so don’t be discouraged! Some contributors may sign off on the pull request right away, others may have more detailed comments or feedback. This is a necessary part of the process in order to evaluate whether the changes are correct and necessary.

Making Updates

To make changes to an existing pull request:
git add my/changed/files
git commit
git push origin my-branch
GitHub will automatically update the pull request.

Handling Conflicts

If a git conflict arises, synchronize your branch with upstream:
git fetch upstream HEAD
git rebase FETCH_HEAD
git push --force-with-lease origin my-branch
The git push --force-with-lease command is one of the few ways to delete history in git. Before you use it, make sure you understand the risks. If in doubt, ask for guidance in the pull request.

Approval and Request Changes Workflow

All pull requests require “sign off” in order to land. While such requests are intended to be helpful, they may come across as abrupt or unhelpful, especially requests to change things that do not include concrete suggestions on how to change them.
Try not to be discouraged. If you feel that a particular review is unfair, say so, or contact one of the other contributors in the project and seek their input.

Step 10: Landing

In order to land, a pull request needs to be:
  • Reviewed and approved by at least two Node.js Collaborators (one collaborator approval is enough if the pull request has been open for more than 7 days)
  • Pass a CI (Continuous Integration) test run
  • Have no objections from other contributors
When a collaborator lands your pull request, they will post a comment to the pull request page mentioning the commit(s) it landed as.
GitHub might show the pull request as Closed at this point, but don’t worry. If you look at the branch you raised your pull request against, you should see a commit with your name on it. Congratulations and thanks for your contribution!

Reviewing Pull Requests

Review Philosophy

All Node.js contributors who choose to review and provide feedback on Pull Requests have a responsibility to both the project and the individual making the contribution. Reviews and feedback must be:
  • Helpful: Provide actionable suggestions
  • Insightful: Share expertise and knowledge
  • Constructive: Focused on improving the contribution
  • Respectful: Follow the Code of Conduct
Reviews that are dismissive or disrespectful of the contributor or any other reviewers are strictly counter to the Code of Conduct.

Review Goals

When reviewing a pull request, the primary goals are:
  1. For the codebase to improve
  2. For the person submitting the request to succeed
Even if a pull request does not land, the submitters should come away from the experience feeling like their effort was not wasted or unappreciated. Every pull request from a new contributor is an opportunity to grow the community.

Review a Bit at a Time

Do not overwhelm new contributors. Focus first on the most significant aspects:
  1. Does this change make sense for Node.js?
  2. Does this change make Node.js better, even if only incrementally?
  3. Are there clear bugs or larger scale issues that need attending to?
  4. Is the commit message readable and correct?
Nits (requests for small changes that are not essential) are fine, but try to avoid stalling the pull request. Most nits can typically be fixed by the Node.js collaborator landing the pull request.

Be Aware of the Person Behind the Code

Be aware that how you communicate requests and reviews in your feedback can have a significant impact on the success of the pull request. The goal is not just having good code - it’s also ensuring contributors want to continue participating in the project.

Respect the Minimum Wait Time

There is a minimum waiting time which we try to respect for non-trivial changes:
  • Non-trivial changes: Must be left open for at least 48 hours
  • Trivial changes: Limited to small formatting changes or fixes to documentation, may be landed within the minimum 48 hour window

Abandoned or Stalled Pull Requests

If a pull request appears to be abandoned or stalled:
  1. Check with the contributor to see if they intend to continue
  2. Ask if they would mind if you took it over
  3. Give the original contributor credit in the commit log
If a pull request has been inactive for more than six months, add the stalled label. This triggers automation that adds a comment explaining the pull request may be closed for inactivity.

Approving a Change

Any Node.js core collaborator is authorized to approve any other contributor’s work. Collaborators are not permitted to approve their own pull requests. Collaborators indicate approval by:
  • Using GitHub’s Approval Workflow (preferred)
  • Leaving an LGTM (“Looks Good To Me”) comment

Continuous Integration Testing

All pull requests that contain changes to code must be run through continuous integration (CI) testing at https://ci.nodejs.org/. Only Node.js core collaborators and triagers can start a CI testing run.

Notes

Commit Squashing

In most cases, do not squash commits that you add to your pull request during the review process. When the commits in your pull request land, they may be squashed into one commit per logical change. Metadata will be added to the commit message including:
  • Links to the pull request
  • Links to relevant issues
  • Names of the reviewers

Getting Approvals for Your Pull Request

A pull request is approved either by saying LGTM (“Looks Good To Me”) or by using GitHub’s Approve button.
After you push new changes to your branch, you need to get approval for these new changes again, even if GitHub shows “Approved” from previous reviews.

Waiting Until the Pull Request Gets Landed

A pull request needs to stay open for at least 48 hours from when it is submitted, even after it gets approved and passes the CI. This is to make sure that everyone has a chance to weigh in.
If the changes are trivial, collaborators may decide it doesn’t need to wait. A pull request may well take longer to be merged in. All these precautions are important because Node.js is widely used, so don’t be discouraged!

Subsystems

When writing commit messages, prefix with the appropriate subsystem:
  • lib/*.js (assert, buffer, etc.)
  • build
  • doc
  • lib / src
  • test
  • tools
You can find the full list of supported subsystems in the nodejs/core-validate-commit repository.

Resources