| # 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 or sometimes impossible to review |
| in the GitHub UI. 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 new dependencies. |
| |
| 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 **5000 lines** of net |
| change (lines added minus lines deleted). This threshold applies across all |
| files in the pull request, including changes in `deps/`, `test/`, `doc/`, |
| `lib/`, `src/`, and `tools/`. |
| |
| Any pull request that adds a new subsystem, e.g. `node:foo` or `node:foo/bar`, |
| is automatically considered a large pull request and subject to the same rules. |
| |
| 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). |
| * Test-only refactoring that involves no functional changes. |
| 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. |
| Large pull requests from non-collaborators will be closed unless it has been discussed |
| in an issue and has 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. |
| * 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. |
| |
| ## Avoiding large pull requests |
| |
| Contributors should always consider whether a large pull request can be split |
| into smaller, independently reviewable pull requests. 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. |
| |
| ### Strategies for reducing the review length in single pull requests |
| |
| Large pull requests may involve a longer review process that becomes practically |
| impossible to track on GitHub due to UI limitations. These strategies help reduce |
| the review length in a single pull request. |
| |
| * Open an issue first to confirm a substantial change is indeed desired in core |
| to reduce lengthy discussions unrelated to the implementation in the pull request. |
| * Use proposal issues, RFCs, design documents, or other types of venues to |
| explore high-level design and cross-cutting concerns. |
| * Keep the initial change provisional to reduce the thoroughness required in a |
| single pull request. Gate premature changes behind build/runtime flags, or apply |
| `dont-land-*` labels to avoid releasing the initial changes until it has been more |
| thoroughly tested and iterated in follow-up pull requests. |
| * Leave non-blocking issues (e.g. stylistic preferences) to follow-up pull requests |
| with a TODO comment in appropriate places. |
| |
| ### 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. |