RFC: PR limits for non-committers

Hey folks, we’ve had a lot of conversations about AI and LLMs and how they are changing the open source development landscape, in Nikita’s RFC,in the contributor policy refresh RFC, and in person at the US LLVM developer meeting. It’s very clear that contributors, and especially maintainers, are tired of dealing with the overload of PRs, and we should act quickly to adjust the policy.

Therefore, I am proposing that we enable pull request limits for contributors without write access to our repos. This is a drastic action, but I think that’s what the situation calls for. The idea is to leave the door open so anyone can upload a few fixes or start small, while preventing new contributors from casting a wide net by opening 20 PRs at once in search of reviewers willing to rubber stamp small, functionally correct fixes. Fixing bugs is useful, but the fixes are clogging our open PR dashboards and causing maintainer burnout.

At the developer meeting, I started with a position of “we should close all non-contributor PRs”, but then I slept on it, moderated a bit, proposed a PR limit of one, and have gotten some feedback pushing me in the direction of raising the PR limit further. So, my current proposal is a limit of 2 open PRs at a time.

My hope is that we will quickly (one week) have consensus that we need some limit, I’ll get some directional feedback on what the limit should be, we’ll enable it before the next project council meeting on Dec 2, and we’ll adjust it again next year.

Nikita and I did some quick analysis of currently open PRs from non-committers, tabulated here. At the time the data was gathered, there were 3661 open PRs from 2010 non-committer contributors.

PR limit PRs blocked/closed % of open PRs Contributors affected % of contributors
1 1651 45.1% 629 31.3%
2 1022 27.9% 289 14.4%
3 733 20.0% 164 8.2%
4 569 15.5% 109 5.4%
5 460 12.6% 79 3.9%

The data has a very long tail. 85% of non-committer contributors open at most 2 PRs. A PR limit of 2 would have reduced our open PR load by 28%, but it would only affect 14.4% or 289 individual contributors. A lower PR limit will create more friction, and push non-committers to apply for commit access earlier.

I want folks to see this as a first step in a longer process of overhauling our contributor process. Some of the feedback on the refresh RFC was that it was doing too many things at once, so this RFC is peeling off the first step to get formal approval.

Many of us are working on follow-up proposals to create new paths to becoming a contributor involving sponsorship, Discord, in person meetings, and other kinds of paths that add back necessary friction to the process. These new processes will help us assess if new contributors are truly interested in learning and growth, and not just padding their resume through automation and labor extraction.

Here is a screenshot of the specific moderation setting panel in question, to provide some transparency on what is possible. The basic control here is “Limit open pull requests from users without write access”, and then there are additional knobs that can be tweaked, but I think the defaults are acceptable.


With that out of the way, here is a poll for providing structured feedback for what you think the limit should be. Otherwise, feel free to share your thoughts below.

  • no limit
  • 1
  • 2
  • 3
  • 4
  • 5
  • 6 or higher
0 voters
22 Likes

0 isn’t an option in the poll, which may bias the results.

This is quite literally the ‘least’ we could do here for maintainers. I think we need to do something like this ASAP.

5 Likes

Thank you Reid for doing the extra work and spending so much time during the LLVM dev meeting discussing this.

This feels like a good compromise while we take the time to come up with a better solution(s). It take some of the pressure off the maintainers who are feeling this the worst.

3 Likes

Thank you for continuing to work towards solutions to this problem! I think this is a reasonable step in the right direction, though I’m guessing we’ll need to tweak the rules as we get feedback once they’re implemented in practice.

I think this is reasonable; it may seem like a very quick turnaround time for an RFC, but the current situation needs addressing quickly IMO. We can iterate on changes to (or reverting) this just as quickly if need be.

5 Likes

I think this is a really good start.

I was concerned that we could have problems where a reviewer asks for some part of the PR to be split off into a preliminary change, but it looks like that could easily be handled by converting the original PR to a draft until the follow-up prerequisite PR is merged.

This doesn’t address my main concern, which is the way that newly opened issues are quickly pounced upon by new contributors. The number of different non-committer contributors in your data is alarming, and it reflects my experience. The perceived barrier to contribution has been lowered to the point that I’m nervous about opening issues to track problems that I don’t want to have to defend from inexperienced contributors. I suppose this is part of the “follow-up proposals” that you mentioned, but I would really love to see some kind of process where new contributors were required to find a mentor within the community who would help the new contributor learn community expectations.

10 Likes

strong +1 to the approach, and acting fairly quickly. I might quibble on the number - I’d probably start slightly high, and then drop it in stages with that intent clearly communicated, but this is a small tweak to a sensible proposal. Something is much better than nothing here.

So i am torn here. The reason is I have a few folks I manage which have at a few PR open that is not getting any reviews on them in an area which seems not getting any attention. But at the same time, I understand the reason for the limit. They don’t have access either. So limiting these folks due to the area not getting any attention seems unfair. Especially since their other patches do seems to being reviewed.

How do we allow this case still?

1 Like

All of this seems really high bar to join as a contributor. Especially from a few contributors that might just need one or two issues fixed (like say a bug which shows up from their open source project and yes gcc has got this contribution type recently).

That said there is no one single answer here i think.

Maybe it is more pick and chose which PRs to get reviewed which is already happening. Maybe it is reviewers and code owners need to be more vocal of what is the direction things could be. Maybe there needs more overall project directing rather than everyone doing their thing. …

As a relatively new contributor, I strongly support the idea.

2 Likes

+1;At present, I think it is quite easy for contributors to the LLVM community to obtain committer access (three merged PRs and two reviewers). Therefore, IMO, limiting non-committer contributors to two open PRs at a time is reasonable. After all, if someone has three PRs and intends to continue contributing in the long term, perhaps they should apply for committer access?

1 Like

+1 in general, as I agree with the issue, but I have a concern about edge cases.

TL;DR: what are we going to do where multiple PRs beyond the limit by the same contributor without commit access are warranted?

I’ve got one specific example in mind, where somebody without commit access is currently adding a new JSON output format to llvm-dwarfdump. This will require a new tool option, likely some refactoring, and implementation of the format for each of the different DWARF debug section types. Each of those should land as a separate PR. However, until we have at least one or two of the JSON output PRs ready and approved, it doesn’t make sense to add the tool option, but without the tool option we cannot have testing for the JSON output. Thus the sequence I proposed was something like this:

  1. Create PR for tool option.
  2. Create 1+ PRs for necessary refactorings.
  3. Create a PR for the first section output, depending on the PRs from 1 and 2.
  4. Once the PR in 3 is approved, land the PRs in the stack.

Given the unknown number of PRs that might be needed for clean individual commits for the refactoring, an open PR limit of any given number might not be enough for such cases. Equally, it doesn’t make sense for me to merge the contributor’s first PRs without the subsequent implementation: doing so could lead to end-user confusion, should the actual implementation not land, or worse, lead to a long-term dead option if the contributor abandons the project without saying anything, then others not remembering to delete the unused option.

If it’s a case of “this PR is too big, we need to split it up”, we potentially might be able to close the original and create the smaller pieces, but what about the case where the work only makes sense when it is all done?

1 Like

+1. I think a limit of two open PRs for non-committers is a reasonable compromise. It should help ease the burden on maintainers while keeping the door open for new contributors.

Wouldn’t “draft PRs” just work for these cases?

1 Like

Wouldn’t “draft PRs” just work for these cases?

Probably, as long as draft PRs still generate notifications for changes for those subscribed to them (and of course assuming they are excluded from the limit). We’d probably need to document this option though, because it isn’t necessarily obvious.

Given this analysis, does this really need a poll? “2” seems to be the correct number backed by data.

Strong support from me for this too. There seem to be people who just run an agent and submit 10 PRs at once, sometimes even in completely unrelated subsystems. This is pretty much the definition of the word “extractive” and a limit would really help with that!

1 Like

I think this will also really benefit new contributors. It is discouraging when I start working on an issue (especially a good first issue) and before I can solve it an LLM PR drops on the issue.

My bad, sorry I forgot that option, the “we don’t take anonymous PRs” option.

I think the non-zero limit is intended to handle the “fix a bug or two” cases, which I agree, are real, they exist. With LLMs, if you hit a bug, why not build the project from source and try to fix it and send the fix and say if it worked?

I think with the volume of PRs where it is, we actually need a way to clear the backlog and mark unreviewed stale PRs as inactive. I’m not saying these changes are low quality, but part of why they haven’t been reviewed is that… the job of reviewing is never-ending and challenging, and the PR limit may actually make the queue more manageable, resulting in prompter, faster feedback because there isn’t a huge backlog of stale PRs to look at.

I think if you’re in this kind of relationship with a contributor, you are practically already their sponsor/mentor. You’re basically saying “I think this contributor is valuable, I want to give them the ability to open multiple PRs, and I’ll take responsibility for reviewing them.”

I think more work needs to be done to hash out the details, but basically, if you want to use native github PR stacks, you need to have repo write access to create user branches, so at that point, this person has to be a committer.

I think openness to adding new contributors has been a key ingredient in the success of LLVM, and we should continue in that tradition, even if it looks a bit different than it used to.

:slight_smile: Well, different people have different views about the ideal tradeoff between spam and openness, so it’s an opportunity to share those views.

1 Like

Its not a tradition, its part of the fabrics of what LLVM is all about. Without new contributors, there is no future in any open source project. I’ve given countless newcomer orientations at this point and decisions should lead with pathways instead of just barriers for newcomers. For every proposal that places a limit, I would want to be very careful in that decision.

Again, openness is not a dirty word. We should always strive to find the right balance between removing barriers, but also providing newcomers with the right support and guidance.

Regarding the proposal, is the goal to reduce the number of PRs or to reduce AI generated PRs?

Also, have you looked at how the LLVM project is growing? Given the majority are at 1-2, I don’t think that a cap is going to address the issue. Have you looked at how people open PRs overtime?

Also, what is the pathway for folks who hit a cap and need help for cases that don’t fit your mould. What is the plan to evaluate this over time?

I also don’t think this the proper way to determine consensus. We have never done polls before and if you are planning to do a “vote” then you need proper time and attention to the matter.

3 Likes