Merge remote-tracking branch 'origin/doc/large-pull-requests' into doc/large-pull-requests

Author: mcollina

doc/contributing/collaborator-guide.md

@@ -132,7 +132,7 @@ 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.
+Pull requests that exceed 5000 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

doc/contributing/large-pull-requests.md

@@ -14,21 +14,26 @@
 
 ## 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.
+Large pull requests are difficult to review and can be 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 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/`.
+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.
@@ -42,6 +47,7 @@ even if they exceed the line threshold:
 * 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.
@@ -51,9 +57,9 @@ benefit from the additional requirements described here.
 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.
+size. Collaborators should close large pull requests from non-collaborators
+unless the proposed change has been discussed in an issue first and a
+collaborator has agreed to champion the work.
 
 ## Requirements
 
@@ -62,17 +68,17 @@ 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 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.
+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
 
@@ -82,8 +88,8 @@ 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.
+* Include a testing plan explaining how the change has been validated and how
+  reviewers can verify the behavior.
 
 ### Approval requirements
 
@@ -102,11 +108,11 @@ When a large pull request adds or modifies a dependency in `deps/`:
   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.
+  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
 
@@ -117,8 +123,27 @@ into smaller, independently reviewable pieces. Strategies include:
 * 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.
+include the implementation, tests, and documentation needed for that piece to
+stand on its own.
+
+### Strategies for reducing review length in a single pull request
+
+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 that a substantial change is indeed desired
+  in core, reducing lengthy discussions unrelated to the implementation in the
+  pull request.
+* Use proposal issues, RFCs, design documents, or other 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 or runtime flags,
+  or apply `dont-land-*` labels to avoid releasing the initial changes until
+  they have 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
 
@@ -155,4 +180,5 @@ should:
   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.
+* Coordinate with other reviewers to divide the review workload when
+  possible.

doc/contributing/pull-requests.md

@@ -289,7 +289,7 @@ 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
+If your pull request exceeds 5000 lines of changes, see the
 [large pull requests][] guide for additional requirements.
 
 Once opened, pull requests are usually reviewed within a few days.