"Authoring a large PR" contribution guide

[mcollina]

During the London 2026 Collab Summit at Bloomberg's (thanks for hosting!), we identified the need for a new policy describing how to:

1. add a new subsystem
2. doing significant modifications to existing subsystem
3. making things easier to review in case of "heavy" code PRs

I would take on preparing this policy, but I would need input from reviewers on what would make their lives easier.

I've added a list underneath of what I would like to see, feel free to comment underneath and I will update the list

---

Key Requirements for this policy:

• applies to any PR over 3000 (+/-) changes
• requires the same approval path of a semver-major change (2 TSC approvals).
• discourage non-collaborators to open them (ban them)
• require an open issue/design document shared before the work start
• add a review guide in the top description pointing to the key files to review
• applies when adding a new dependency
• ...

[mcollina]

cc @nodejs/tsc @indutny

[jasnell]

5000 is still a bit high. I'd make the limit 3000 or higher.

Everything else is good but I think also a review guide / testing plan should also be required.

[ShogunPanda]

One little remark, maybe unsolveable but I want to raise it anyway.
In case a PR is including a new dependency, (for instance see #62072) do we count the deps folder as well?
I would say "yes", but mostly I want to make sure we properly evaluate this edge case.

[targos]

Yes. Deps are even more sensitive because we don't review the changes.

[ShogunPanda]

Also, do we want to have a single set of rules or apply two different rules?

Like: 3000+ lines if you are not a contributor, 6000+ if you are.

Once again, I don't have a real answer / opinion. Just brainstorming.

[tniessen]

This probably goes without saying, but I think we should include in any such policy a general guideline stating that all contributors should avoid creating such large PRs except in those very rare cases where that is effectively unavoidable.

[joyeecheung]

I think we can place different threshold on different directories, for example

• deps obviously has a lot more leeway or are reviewed differently (we generally review the scripts that pull them in and the whole provenance - as long as the scripts and process is sound there's not much point going through the artifact). I'd say the limit can be very loose as long as it's reproducible by the scripts that pull them in, and the threshold should be on the scripts/floated patches instead.
• test generally has a lot of boilerplate to bloat the diff, but an experience reviewer's brain can just scan the boilerplate very quickly so the size is less a problem, and tests are lower stakes (as long as they look legit and not malicious, there's very limited harm they can do other than flaking/breaking more often), I think something like <5K-10K is maximum for me for things that need to be tested thoroughly (there's a downside that if we place a low threshold, people would just avoid writing as many tests because once you reach a certain line, more tests generally are optional, then we get reduced coverage in return which is not great - though we can also just ask them to split the additional tests in a follow-up).
• doc is in the same regard as test, though the harm is even more benign. But they take some time to reviewe for a non-English speaker. I don't think they tend to bloat the diff though, more often than not we are asking people to write more docs, not less..
• tools have higher stakes than tests, but still there's very limited harms they can do, it's mostly if it's copying external tools we need to be careful about provenance
• lib obvious requires a lot more care, and src maybe even more so. I think 3K-5K is my personal maximum review capacity for those, but I definitely prefer <1K whenever possible

[jasnell]

Yes. Deps are even more sensitive because we don't review the changes.

Yes, I think also we need to explicitly always require that deps changes themselves HAVE to be in a separate commit in the PR

@tniessen:

... a general guideline stating that all contributors should avoid creating such large PRs except in those very rare cases where that is effectively unavoidable.

I don't think I would go that far. Language explaining the risks associated with such large PRs I think should be sufficient... e.g. they'll almost never get a fully adequate review, they're likely to sit for a long time if they get review at all etc. For some subsystems large PRs are just naturally necessary, especially in early active development stages. I'm absolutely fine with saying that new contributors / anyone who doesn't have an established history with the project is strongly discouraged or even not allowed to make large PR contributions but I don't think it's necessary to explicitly say avoid it for existing contributors.

@mcollina:

... shared before the work start

This is not always possible as often the design doc takes shape as the work progresses. Requiring that a design doc be shared before the work starts gets us back into ineffective Talk Forever And Never Get Work Done RFC territory. I would say that a design doc must be shared and available when the PR is opened, not before the work is started. As a reviewer, before I start digging into the complex code I want the author's explanation of what the code is doing and for large PRs that needs to be as detailed as possible.

[jasnell]

I think we should also consider formalizing the Feature Fork/Branch concept for extremely complex large PRs that take shape over time. We've leveraged this successfully in the past for major new subsystems. We'd need some criteria for what qualifies.

[tniessen]

For some subsystems large PRs are just naturally necessary, especially in early active development stages.

I believe this falls under the aforementioned "rare cases where that is effectively unavoidable" -- e.g., when adding a feature as complex and important as QUIC :-)

[alexweej]

https://github.github.com/gh-stack/ when it's generally available should be useful?