Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ dependencies, and tools contained in the `nodejs/node` repository.
* [Setting up your local environment](./doc/contributing/pull-requests.md#setting-up-your-local-environment)
* [The Process of Making Changes](./doc/contributing/pull-requests.md#the-process-of-making-changes)
* [Reviewing Pull Requests](./doc/contributing/pull-requests.md#reviewing-pull-requests)
* [Large Pull Requests](./doc/contributing/large-pull-requests.md)
* [Notes](./doc/contributing/pull-requests.md#notes)

## Developer's Certificate of Origin 1.1
Expand Down
4 changes: 4 additions & 0 deletions doc/contributing/collaborator-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ Pay special attention to pull requests for dependencies which have not
been automatically generated and follow the guidance in
[Maintaining Dependencies](https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#updating-dependencies).

Pull requests that exceed 3000 lines of changes have additional requirements.
See the [large pull requests][] guide.

In some cases, it might be necessary to summon a GitHub team to a pull request
for review by @-mention.
See [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker).
Expand Down Expand Up @@ -1068,6 +1071,7 @@ need to be attached anymore, as only important bugfixes will be included.
[git-node]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md
[git-node-metadata]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-metadata
[git-username]: https://help.github.com/articles/setting-your-username-in-git/
[large pull requests]: large-pull-requests.md
[macos]: https://github.com/orgs/nodejs/teams/platform-macos
[node-core-utils-credentials]: https://github.com/nodejs/node-core-utils#setting-up-credentials
[node-core-utils-issues]: https://github.com/nodejs/node-core-utils/issues
Expand Down
158 changes: 158 additions & 0 deletions doc/contributing/large-pull-requests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# Large pull requests

* [Overview](#overview)
* [What qualifies as a large pull request](#what-qualifies-as-a-large-pull-request)
* [Who can open a large pull request](#who-can-open-a-large-pull-request)
* [Requirements](#requirements)
* [Detailed pull request description](#detailed-pull-request-description)
* [Review guide](#review-guide)
* [Approval requirements](#approval-requirements)
* [Dependency changes](#dependency-changes)
* [Splitting large pull requests](#splitting-large-pull-requests)
* [Feature forks and branches](#feature-forks-and-branches)
* [Guidance for reviewers](#guidance-for-reviewers)

## Overview

Large pull requests are difficult to review thoroughly. They are likely to sit
for a long time without receiving adequate review, and when they do get reviewed,
the quality of that review is often lower due to reviewer fatigue. Contributors
should avoid creating large pull requests except in those cases where it is
effectively unavoidable, such as when adding a major new subsystem.

This document outlines the policy for authoring and reviewing large pull
requests in the Node.js project.

## What qualifies as a large pull request

A pull request is considered large when it exceeds **3000 lines** of combined
additions and deletions. This threshold applies across all files in the pull
request, including changes in `deps/`, `test/`, `doc/`, `lib/`, `src/`, and
`tools/`.

Changes in `deps/` are included in this count. Dependency changes are
sensitive because they often receive less scrutiny than first-party code.

The following categories of pull requests are **excluded** from this policy,
even if they exceed the line threshold:

* Routine dependency updates (e.g., V8, ICU, undici, uvwasi) generated by
automation or performed by collaborators following the standard dependency
update process.
* Web Platform Tests (WPT) imports and updates.
* Other bot-issued or automated pull requests (e.g., license updates, test
fixture regeneration).

These pull requests already have established review processes and do not
benefit from the additional requirements described here.

## Who can open a large pull request

Large pull requests may only be opened by existing
[collaborators](https://github.com/nodejs/node/#current-project-team-members).
Non-collaborators are strongly discouraged from opening pull requests of this
size. Collaborators should close large pull requests from non-collaborators and
direct the author to discuss the proposed changes in an issue first, and to
find a collaborator to champion the work.

## Requirements

All large pull requests must satisfy the following requirements in addition to
the standard [pull request requirements](./pull-requests.md).

### Detailed pull request description

The pull request description must provide sufficient context for reviewers
to understand the change. The description should explain:

* The motivation for the change.
* The high-level approach and architecture.
* Any alternatives that were considered and why they were rejected.
* How the change interacts with existing subsystems.

A thorough pull request description is sufficient. There is no requirement
to produce a separate design document, although contributors may choose to
link to a GitHub issue or other discussion where the design was developed.

### Review guide

The pull request description must include a review guide that helps reviewers
navigate the change. The review guide should:

* Identify the key files and directories to review.
* Describe the order in which files should be reviewed.
* Highlight the most critical sections that need careful attention.
* Include a testing plan explaining how the change has been validated and
how reviewers can verify the behavior.

### Approval requirements

Large pull requests follow the same approval path as semver-major changes:

* At least **two TSC member approvals** are required.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we enforce commit-queue bot to check this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. Maybe @aduh95 knows?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a blocker for this landing?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking

* The standard 48-hour wait time applies. Given the complexity of large pull
requests, authors should expect and allow for a longer review period.
* CI must pass before landing.

### Dependency changes

When a large pull request adds or modifies a dependency in `deps/`:

* Dependency changes should be in a **separate commit** from the rest of the
pull request. This makes it easier to review the dependency update
independently from the first-party code changes. When the pull request is
squashed on landing, the dependency commit should be the one that carries
the squashed commit message, so that `git log` clearly reflects the
overall change.
* The provenance and integrity of the dependency must be verifiable.
Include documentation of how the dependency was obtained and how
reviewers can reproduce the build artifact.

## Splitting large pull requests

Contributors should always consider whether a large pull request can be split
into smaller, independently reviewable pieces. Strategies include:

* Landing foundational internal APIs first, then building on top of them.
* Landing refactoring or preparatory changes before the main feature.

Each pull request in a split series should remain self-contained: it should
include the implementation, tests, and documentation needed for that piece
to stand on its own.

### Feature forks and branches

For extremely large or complex changes that develop over time, such as adding
a major new subsystem, contributors should consider using a feature fork.
This approach has been used successfully in the past for subsystems like QUIC.

The feature fork must be hosted in a **separate GitHub repository**, managed
by the collaborator championing the change. The repository can live in the
[nodejs organization](https://github.com/nodejs) or be a personal repository
of the champion. The champion is responsible for coordinating development,
managing access, and ensuring the fork stays up to date with `main`.

A feature fork allows:

* Incremental development with multiple collaborators.
* Review of individual commits rather than one monolithic diff.
* CI validation at each stage of development.
* Independent issue tracking and discussion in the fork repository.

When the work is ready, the final merge into `main` via a pull request still
requires the same approval and review requirements as any other large pull
request.

## Guidance for reviewers

Reviewing a large pull request is a significant time investment. Reviewers
should:

* Read the pull request description and review guide before diving into the
code.
* Focus review effort on `lib/` and `src/` changes, which have the highest
impact on the runtime. `test/` and `doc/` changes, while important, are
lower risk.
* Not hesitate to request that the author split the pull request if it can
reasonably be broken into smaller pieces.
* Coordinate with other reviewers to divide the review workload when possible.
4 changes: 4 additions & 0 deletions doc/contributing/pull-requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ 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, but feel free to skip parts if you're not sure what to put.

If your pull request exceeds 3000 lines of changes, see the
[large pull requests][] guide for additional requirements.

Once opened, pull requests are usually reviewed within a few days.

To get feedback on your proposed change even though it is not ready
Expand Down Expand Up @@ -611,6 +614,7 @@ More than one subsystem may be valid for any particular issue or pull request.
[guide for writing tests in Node.js]: writing-tests.md
[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment
[https://ci.nodejs.org/]: https://ci.nodejs.org/
[large pull requests]: large-pull-requests.md
[maintaining dependencies]: ./maintaining/maintaining-dependencies.md
[nodejs/core-validate-commit]: https://github.com/nodejs/core-validate-commit/blob/main/lib/rules/subsystem.js
[pull request template]: https://raw.githubusercontent.com/nodejs/node/HEAD/.github/PULL_REQUEST_TEMPLATE.md
Expand Down
Loading