Skip to content

Latest commit

 

History

History
140 lines (104 loc) · 5.89 KB

File metadata and controls

140 lines (104 loc) · 5.89 KB

Large pull requests

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.

Who can open a large pull request

Large pull requests may only be opened by existing collaborators. 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.

Design document

A design document or detailed issue must be available when the pull request is opened. The design document 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.

The design document can be a GitHub issue, a Markdown file in the repository, or an external document linked from the pull request description. It does not need to be shared before the work starts, but it must be available for reviewers when the pull request is opened.

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 must 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.
  • 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.
  • Separating test additions from implementation changes.
  • Splitting documentation into its own pull request.
  • Landing refactoring or preparatory changes before the main feature.

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 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 design document 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.