GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: change line threshold and reword subsystem test in large PR policy by Renegade334 · Pull Request #65685 · nodejs/node

Conversation

@Renegade334

@Renegade334 Renegade334 commented Aug 31, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Proposes two material changes to the large PR policy:

Firstly, the current line calculation (abs(added - deleted)) only really covers PRs that add or remove large featuresets. Changes to existing lines are calculated as +1/-1 by Git, so a PR that edits a million lines in-place has a "net size" of 0 and doesn't count as a large PR by this definition. A PR that makes 5,000 changes to existing code is no less challenging or time-consuming to review than one that adds 5,000 of new code, and we should include these under the policy.

This is a change from the approach discussed in #62829, so I'm anticipating that this will generate some discussion. However, the case example provided (splitting a flaky test into several subtests) would already be excluded under this policy, and 5,000 is no small threshold; it's easy to miss (or hide) something in a massive PR that purports to be doing something mundane.

Secondly, the policy wording states "add a new subsystem", but really intends to state "add a new public module"; the given example of adding a node:foo/bar module would not create a new core subsystem (it would come under foo) but should still be considered a large PR. Altered the definition accordingly.

I've also moved the paragraph that clarifies the /deps count to underneath the paragraph that it elaborates on, for greater readability.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Renegade334
Renegade334 requested a review from mcollina August 31, 2026 15:44
@nodejs-github-bot

nodejs-github-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/tsc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. label Aug 31, 2026
@Renegade334

Renegade334 commented Aug 31, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

For real-world context, this is a list of all of the commits in the last two years that would have triggered the line threshold after this change, but would not have done so before:

Details

9cf2ce7 deps: update perfetto to 57.2
f7804f7 test: update WPT for WebCryptoAPI to 82c3d9069c
5cbedc0 deps: update nghttp2 to 1.70.0
eb1a49b deps: update ada to 4.0.0
36bcfa7 deps: upgrade npm to 11.17.0
03f625c deps: update ngtcp2 to 1.23.0
d27ab8c deps: update zlib to 1.3.2.1-motley-3246f1b
17d03b5 deps: update sqlite to 3.53.0
586403f deps: update ngtcp2 to 1.22.0
9580d58 deps: update sqlite to 3.51.3
3dff7ef deps: update sqlite to 3.52.0
76215dc build, doc: use new api doc tooling
44bd030 deps: upgrade npm to 11.8.0
15bdf38 deps: update nghttp3 to 1.13.1
67527c4 deps: update c-ares to v1.34.6
11813f9 tools: update cpplint to 2.0.2
00a8377 deps: update brotli to 1.2.0
1b22f60 deps: upgrade npm to 11.6.2
5d843c9 deps: upgrade npm to 11.6.1
ac131bd deps: update undici to 7.16.0
f03e90a deps: upgrade npm to 11.3.0
ab9660b deps: update icu to 77.1
72aa2ea deps: update ada to v3.2.1
063e67c deps: update ada to v3.0.1
261624b deps: update simdjson to 3.11.6
744c2f5 inspector: roll inspector_protocol
25b22e4 deps: upgrade npm to 11.0.0
062ae6f src, quic: refine more of the quic implementation
2a3fc98 deps: update nghttp3 to 1.6.0
839ad8b deps: upgrade npm to 10.9.2
47b80c2 deps: update undici to 7.0.0
c1b3531 deps: update sqlite to 3.47.0
ece37bc deps: update nghttp2 to 1.64.0
4dc76a9 deps: update simdutf to 5.6.0

Most of these are automated dependency updates that would not qualify anyway. However, the three human PRs in this list are prime examples of what should really be considered a large PR:

  • 76215dc changed the entire documentation system.
  • 744c2f5 was a large dependency update, which falls within the scope of the large PR guidance.
  • 062ae6f was a large core PR changing much of the quic implementation.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca
lpinca approved these changes Aug 31, 2026
View reviewed changes
@Renegade334
doc: change line threshold and reword subsystem test in large PR policy
d6a7776
Signed-off-by: Renegade334 <[email protected]>
@Renegade334
Renegade334 force-pushed the large-pr-threshold branch from 01ce2c5 to d6a7776 Compare September 4, 2026 22:03
@Renegade334

Renegade334 commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Would anyone else from @nodejs/tsc like to weigh in on this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
@mcollina mcollina Awaiting requested review from mcollina

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@Renegade334 @nodejs-github-bot @lpinca
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.