Repository navigation
"Authoring a large PR" contribution guide #62752
Description
Activity
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.
Reacted by Matteo Collina, Ilyas Shabi, Jordan Harband, Chengzhong Wu and Nikita SkovorodaOne 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.Reacted by Matteo CollinaYes. Deps are even more sensitive because we don't review the changes.
Reacted by Marco Ippolito, Richard Lau, Rafael Gonzaga and Chengzhong WuAlso, 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.
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.
Reacted by Paolo Insogna, Rafael Gonzaga, Marco Ippolito, Jordan Harband and Chengzhong WuI think we can place different threshold on different directories, for example
depsobviously 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.testgenerally 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).docis in the same regard astest, 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..toolshave 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 provenancelibobvious requires a lot more care, andsrcmaybe even more so. I think 3K-5K is my personal maximum review capacity for those, but I definitely prefer <1K whenever possible
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
... 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.
... 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.
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.
Reacted by Tobias NießenFor 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 :-)
https://github.github.com/gh-stack/ when it's generally available should be useful?
Reacted by Matteo Collina- added a commit that references this issue
on May 11, 2026 - added a commit that references this issue
on Jun 18, 2026 - added a commit that references this issue
on Jul 29, 2026 - added a commit that references this issue
on Aug 12, 2026
During the London 2026 Collab Summit at Bloomberg's (thanks for hosting!), we identified the need for a new policy describing how to:
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: