Skip to content

Commit 28a1d11

Browse files
committed
doc: add large pull requests contributing guide
Fixes: #62752
1 parent da92246 commit 28a1d11

File tree

1 file changed

+140
-0
lines changed

1 file changed

+140
-0
lines changed
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Large pull requests
2+
3+
* [Overview](#overview)
4+
* [What qualifies as a large pull request](#what-qualifies-as-a-large-pull-request)
5+
* [Who can open a large pull request](#who-can-open-a-large-pull-request)
6+
* [Requirements](#requirements)
7+
* [Design document](#design-document)
8+
* [Review guide](#review-guide)
9+
* [Approval requirements](#approval-requirements)
10+
* [Dependency changes](#dependency-changes)
11+
* [Splitting large pull requests](#splitting-large-pull-requests)
12+
* [Feature forks and branches](#feature-forks-and-branches)
13+
* [Guidance for reviewers](#guidance-for-reviewers)
14+
15+
## Overview
16+
17+
Large pull requests are difficult to review thoroughly. They are likely to sit
18+
for a long time without receiving adequate review, and when they do get reviewed,
19+
the quality of that review is often lower due to reviewer fatigue. Contributors
20+
should avoid creating large pull requests except in those cases where it is
21+
effectively unavoidable, such as when adding a major new subsystem.
22+
23+
This document outlines the policy for authoring and reviewing large pull
24+
requests in the Node.js project.
25+
26+
## What qualifies as a large pull request
27+
28+
A pull request is considered large when it exceeds **3000 lines** of combined
29+
additions and deletions. This threshold applies across all files in the pull
30+
request, including changes in `deps/`, `test/`, `doc/`, `lib/`, `src/`, and
31+
`tools/`.
32+
33+
Changes in `deps/` are included in this count. Dependency changes are
34+
sensitive because they often receive less scrutiny than first-party code.
35+
36+
## Who can open a large pull request
37+
38+
Large pull requests may only be opened by existing
39+
[collaborators](https://github.com/nodejs/node/#current-project-team-members).
40+
Non-collaborators are strongly discouraged from opening pull requests of this
41+
size. Collaborators should close large pull requests from non-collaborators and
42+
direct the author to discuss the proposed changes in an issue first, and to
43+
find a collaborator to champion the work.
44+
45+
## Requirements
46+
47+
All large pull requests must satisfy the following requirements in addition to
48+
the standard [pull request requirements](./pull-requests.md).
49+
50+
### Design document
51+
52+
A design document or detailed issue must be available when the pull request is
53+
opened. The design document should explain:
54+
55+
* The motivation for the change.
56+
* The high-level approach and architecture.
57+
* Any alternatives that were considered and why they were rejected.
58+
* How the change interacts with existing subsystems.
59+
60+
The design document can be a GitHub issue, a Markdown file in the repository,
61+
or an external document linked from the pull request description. It does not
62+
need to be shared before the work starts, but it must be available for reviewers
63+
when the pull request is opened.
64+
65+
### Review guide
66+
67+
The pull request description must include a review guide that helps reviewers
68+
navigate the change. The review guide should:
69+
70+
* Identify the key files and directories to review.
71+
* Describe the order in which files should be reviewed.
72+
* Highlight the most critical sections that need careful attention.
73+
* Include a testing plan explaining how the change has been validated and
74+
how reviewers can verify the behavior.
75+
76+
### Approval requirements
77+
78+
Large pull requests follow the same approval path as semver-major changes:
79+
80+
* At least **two TSC member approvals** are required.
81+
* The standard 48-hour wait time applies. Given the complexity of large pull
82+
requests, authors should expect and allow for a longer review period.
83+
* CI must pass before landing.
84+
85+
### Dependency changes
86+
87+
When a large pull request adds or modifies a dependency in `deps/`:
88+
89+
* Dependency changes must be in a **separate commit** from the rest of the
90+
pull request. This makes it easier to review the dependency update
91+
independently from the first-party code changes.
92+
* The provenance and integrity of the dependency must be verifiable.
93+
Include documentation of how the dependency was obtained and how
94+
reviewers can reproduce the build artifact.
95+
96+
## Splitting large pull requests
97+
98+
Contributors should always consider whether a large pull request can be split
99+
into smaller, independently reviewable pieces. Strategies include:
100+
101+
* Landing foundational internal APIs first, then building on top of them.
102+
* Separating test additions from implementation changes.
103+
* Splitting documentation into its own pull request.
104+
* Landing refactoring or preparatory changes before the main feature.
105+
106+
### Feature forks and branches
107+
108+
For extremely large or complex changes that develop over time, such as adding
109+
a major new subsystem, contributors should consider using a feature fork.
110+
This approach has been used successfully in the past for subsystems like QUIC.
111+
112+
The feature fork must be hosted in a **separate GitHub repository**, managed
113+
by the collaborator championing the change. The repository can live in the
114+
[nodejs organization](https://github.com/nodejs) or be a personal repository
115+
of the champion. The champion is responsible for coordinating development,
116+
managing access, and ensuring the fork stays up to date with `main`.
117+
118+
A feature fork allows:
119+
120+
* Incremental development with multiple collaborators.
121+
* Review of individual commits rather than one monolithic diff.
122+
* CI validation at each stage of development.
123+
* Independent issue tracking and discussion in the fork repository.
124+
125+
When the work is ready, the final merge into `main` via a pull request still
126+
requires the same approval and review requirements as any other large pull
127+
request.
128+
129+
## Guidance for reviewers
130+
131+
Reviewing a large pull request is a significant time investment. Reviewers
132+
should:
133+
134+
* Read the design document and review guide before diving into the code.
135+
* Focus review effort on `lib/` and `src/` changes, which have the highest
136+
impact on the runtime. `test/` and `doc/` changes, while important, are
137+
lower risk.
138+
* Not hesitate to request that the author split the pull request if it can
139+
reasonably be broken into smaller pieces.
140+
* Coordinate with other reviewers to divide the review workload when possible.

0 commit comments

Comments
 (0)