Dealing with huge PR culture
Posted by semaphoreslimshady42@reddit | ExperiencedDevs | View on Reddit | 71 comments
I joined a company 8 months back and while the team is very smart and capable, they love pushing out monstrous pull requests
The team is fond of AI and I give credit that I think they use it fairly well. The code is good quality, and while they lean heavily onto AI, it's clear that everyone here has a strong programming ability
The situation I face is they're pumping out these monster pull requests. It's usually at least 100 file changes, but 200-300 is normal
They implement entire features, full vertical slices. They'll perform adhoc renames and refractors, so in a 300 file PR, maybe 100 are actual new code, and the others are just tweaks. It is a mono repo too so it's all there.
The team bangs on about delivering smaller units of work, but it's seemingly just talk. One guy, credit to him, will do multiple PRs into an intermediary branch so it's more digestible, so you can review one piece at a time. But otherwise it's just these crazy sized ones
I'm struggling to figure out how to deal with this. If I review them manually it takes a long time, and I'm doubtful of my ability to effectively review after seeing so much code in one go. I have tried to leverage AI to help me distill the PR into something readable... but that gives me a bad taste, getting AI to review AI code.
I'm imagining this is a growing problem now we have AI tools. I don't blame the AI here, it's obviously the developers getting overzealous and wanting to pump out a feature per pull request.
It feels like there's little point raising this to them, because they all acknowledge it's a bad practice to move away from, but none of them actually are... and as a relatively new hire I don't have the social capital to try do anything about it
Feels like my options are either to manually review, to use the AI to help me review, to rubberstamp, or to not engage (not ideal)
Can't imagine I'm the only one facing this. Anyone got any tips?
arihoenig@reddit
Yeah, a PR with more than about 30 files is suspect. If a PR of more than 30 files is truly necessary, then it is a sign that the architecture of the application is broken.
SolarNachoes@reddit
What if a feature requires updating settings, updating settings UI, updating business logic both front end and backend, updating tests, updating docs, updating integrations, etc. it can get a bit crazy.
arihoenig@reddit
Then the architecture is broken. There should be minimal coupling between client and server and there would be no need to PR the client and the server in the same PR.
SolarNachoes@reddit
Sometimes we get assigned a ticket for the full feature. That covers everything backend and frontend even if code is “separate” as in react UI and dotnet API.
arihoenig@reddit
Does the process require that there is one PR per ticket? If so, the process is broken.
gefahr@reddit
Might not be the architecture, could their release management. They mentioned being in a monorepo, this is perhaps the only way to ship atomic changes that you need to land across a large surface simultaneously.
ReflectedImage@reddit
Well I've just joined a place and run into this problem. The architecture is definitely broken at least for the repository I'm working with.
arihoenig@reddit
If you need atomic changes (client and server) then the client and server are coupled and the architecture is broken
gefahr@reddit
Interesting POV. Is Google's SCM is broken?
arihoenig@reddit
Possibly, I don't know what you're referring to.
gefahr@reddit
They famously are a monorepo shop.
arihoenig@reddit
Monorepo is fine if it is just for one monolithic entity, and that entity is inherently monolithic.
gefahr@reddit
..what? Google has thousands of services and products.
arihoenig@reddit
Sure, and they have all of them in one repo?
shigdebig@reddit
The feature should be split into smaller pieces which can be done in smaller prs. Feature flags can hide the feature until its ready so you can ship incremental builds.
SolarNachoes@reddit
If it’s all done by one developer delivered within one sprint, then why?
Far-Consideration939@reddit
I don’t know if it’s as black and white as this. Sometimes a lot of tiny commits or PRs spread across reviewers isn’t as effective review wise if the context is sharded relative to the feature trying to be delivered.
I think this varies by team / company size too. Some small teams just are likely to work on features spanning server and client full stack and there’s a clear outcome that delivering half of doesn’t really achieve.
But I would tend to agree with you when PRs do get to that size there’s likely ways things could have been broken up more effectively. File size isn’t necessarily the only indicator though some refactors touch a lot of files for a low impact/digestible change, so there’s more context to be considered
arihoenig@reddit
New features that span client and server does not imply coupling a necessity to couple the client and server. A properly designed architecture will employ an interface between the client and server that is easily extensible and that is not deployment coupled. For such a system there would be 3 PRs. The first would be any extensions to the interface definition (which can be deployed without breaking either client or server) then secondly either the client or server PR (either of which can be reviewed, approved and deployed either before or after the other).
Far-Consideration939@reddit
Never said otherwise.
arihoenig@reddit
I was responding to the fact that you seemed to imply that poor architecture was fine for small shops.
Far-Consideration939@reddit
Never implied that, but I understand reading is hard.
arihoenig@reddit
"I think this varies by team / company size too. Some small teams just are likely to work on features spanning server and client full stack and there’s a clear outcome that delivering half of doesn’t really achieve."
PRs are not delivery mechanisms. How PRs are structured says nothing about how features are deployed. There is nothing about having 3 PRs that implies that you can't have an atomic deployment. It is just that system should not be architected to force an atomic deployment.
Far-Consideration939@reddit
Either way an atomic deployment isn’t forced though and at your point of arbitration then neither is effectively different
arihoenig@reddit
If the architecture is coupled, then it forces atomic deployment.
gdinProgramator@reddit
Thefuck are these “experienced” comments?
The obvious solution is for you to take this up with mgmt and decide on a solution.
Enforce clean PR - make a clear guide, start rejecting AI slopped shit, start firing people for not doing their jobs.
Dont give a shit about PR’s - LGTM means lets gamble try merging. The fallout is inevitable but that is not your problem. You wait for shit to hit the fan and then you fix it.
ClideLennon@reddit
You can ask your agent to break a PR up into more manageable pieces. It's something they do surprisingly well. Start asking for that.
Ayanrocks@reddit
So how does this work. Do we tell Ai to implement the changes in chunks in the branch and create a new one or is there a way to break a PR that is already created?
ClideLennon@reddit
Yes. I usually say, "Have a look at this branch. It has far to much going on. Break it up into manageable pieces." It usually creates a sequence of dependant branches, each based on the previous branch so the final branch is just your starting branch. You can set the base branch of each PR to be the previous branch instead of main, and you'll get a PRs that are much easier to read.
rwilcox@reddit
Have you tried separate commits, not branches, so you could go commit by commit?
ClideLennon@reddit
I have not. But I imagine there would be no problem. Branches and commits are not functionally different.
Hydrogen_Ion@reddit
Branches are commits when they are merged 🙂
mamaBiskothu@reddit
"Break this PR up into many small PRs" -> say this to your agent
nomadJuice@reddit
Any suggestions or alternatives to Claude code?
mamaBiskothu@reddit
Amp - likely not cheap Augment - cheap and good not great Codex
Thin_Mousse4149@reddit
I have AI split my PRs into reasonable scopes that can be separately merged to master or reasonable stacked PRs that don’t go further than 2 stacks deep. I have a strict 500 lines or less policy with a goal of around 100 lines for each PR.
xt-89@reddit
Implement automated PR stacking. The Dictator and Lieutenants workflow can also help. Integrate these into CI/CD so that it’s automatic.
Urik88@reddit
Do you have a bit more info about this? I get having people responsible for rejecting big pr's, but how'd automated stacking work here? I'm big into stacking pr's for big features (updating Apollo to version 4 took 7 pr's) , but never heard about ways of automating this.
30thnight@reddit
https://github.github.com/gh-stack/
xt-89@reddit
There are tools that will analyze the AST, identify clusters of changes, and place them onto stacked PRs. Adding AI into that helps too. You could write a script or GH Action to trigger all of that.
ClideLennon@reddit
"This branch is too busy. Break it up into smaller more manageable branches." The agents can do it for you. It will return the branch names and the order they are stacked. If you have your agent hooked up to GitHub it can even create the PRs for you and set the base branch to the correct branch instead of main.
neuronexmachina@reddit
TIL:
https://www.martinpickering.com/posts/maginus-flow/dictator-lieutenants/
https://git-scm.com/book/en/v2/Distributed-Git-Distributed-Workflows
David_AnkiDroid@reddit
https://github.github.com/gh-stack/
DevMadness@reddit
I’d like more info here too.
lppedd@reddit
THIS
Stacking is the way to go.
scalablecory@reddit
I’ll usually do refactors or style updates in a separate commit, and each feature in its own commit. I ask agents to do the same.
You can choose to squash or preserve it in history but at review time it can be a useful mechanism.
leakingpointer123@reddit
I’d go even further - create a separate PR for refactors, if the change is not related to the feature in most cases it shouldn’t be there. Otherwise it increases the cost of the revert or an investigation during an outage.
David_AnkiDroid@reddit
If you rebase merge PRs, you don't need a separate PR
likeittight_@reddit
Yes, this is a must. Things might change a lot with agentic flows but breaking the work up like this was the way before and it’s remains the way now.
nickhow83@reddit
A few things we’re doing or are looking to do.
Stacked branches/PRs are a good way to build a feature incrementally and still allows functionality to cascade to the bottom PR. Tools like Graphite can help but to be fair, you can do it with standard Git and ask Claude to update the PR stack.
Leverage LLMs for reviewing code. Set up some rules in a skill. Don’t use this as a crutch but it will definitely help pick out some things. Hell, get a second LLM to look for security holes and Env variables… and while you’re at it, have another LLM to automate the fixes on the PR. (Babysit-pr)
Ask the reviewer to review their own PR and to add comments to potential trade offs and points of interest. Maybe get them to record a loom that walks you through the functionality to prove it works. If someone can’t review their own changes, then why should you?
Ensure the automated tests are robust and well maintained. If they are, and they’re passing then that’s a good indicator. Plus there’s no excuse to not write tests now that we’re all using AI. Also lean into linting and tools like prettier to enforce consistent code styles.
Still with all of this. Please keep a human in the loop and skim read the changes. Look for non obvious things.
caffeinated_wizard@reddit
What’s your role/responsibility on this? You’ll get vastly different answers if you are a senior IC vs a manager.
donniedarko5555@reddit
At my job we kind of soft incentivize it because PRs take ages, and the feature I'm willing to ship in 7 PRs gets blocked longer than the 1 big PR
Solax636@reddit
1 line change - 50 comments, 5000 line change LGTM!
Bicykwow@reddit
Interesting. At my job, I don't even read big PRs, I just auto-reject them.
(Obviously there are exceptions to this, eg certain types of refactoring)
nrith@reddit
That seems backwards.
hallucinagentic@reddit
the ai angle is the main thing nobody is saying directly. when coding was manual, big features naturally got broken up because nobody wanted to sit there typing for 3 days straight. the friction was a feature. ai removes that friction, so now you get entire features in one session and one PR
what actually helped us was moving the work breakdown upstream. before anyone opens their editor or agent, you write down the chunks: "this is the migration PR, this is the service layer PR, this is the api PR." each one gets merged on its own. the agent actually works better this way too because tighter scope means less context drift
the practical move for you specifically: next time you get a monster PR to review, leave a comment like "can we split the refactor out from the feature changes? would make this way easier to review." most people aren't opposed to it, they just default to the path of least resistance. if you do it consistently for a few weeks it becomes normal
the bigger culture shift is getting people to see that the planning step is the actual engineering now. coding is cheap. deciding what gets built in what order, what can be tested independently, where the review boundaries should be, that's the hard part. once the team internalizes that, the PR sizes sort themselves out
Bicykwow@reddit
These 2 statements are not compatible
ugh_my_@reddit
Big PR / Small PR, doesn’t matter. It’s work you need to review, and complaining about the size only makes you look bad. If you need it chopped up then ask for it to be chopped up.
KandevDev@reddit
the PR size lever pulls back on its own once the cost is felt. what worked at my last shop:
(1) PR-size limits in CI: anything over 500 LOC gets auto-labeled "needs justification" and the reviewer dashboard shows them separately. nothing technical prevents merge, just visibility.
(2) the reviewer time is the constraint to make legible. "i will review your 2000-line PR but it will be tomorrow because i need an hour" vs "your 100-line PR i can review now". the team learns the trade-off quickly when their own work depends on quick reviews.
(3) the AI angle is real. agents make it trivially easy to ship 2000-line refactors. the discipline is splitting them into reviewable chunks BEFORE asking the agent, not after. that habit does not develop naturally with AI use, it has to be enforced.
MegaBiteGames@reddit
A pull request with 100+ edited files is simply not human reviewable. Especially if most of that is new code.
You need to talk to your team/management about the theater of these “reviews”. If the size of these PRs is acceptable, then you shouldn’t even have a gating review process. Even the best engineer can not effectively review a 100 file PR, and you’d be better off not wasting their/your time.
The simplest answer is to enforce standards strictly. Does your PR system have a mechanism to reject/block PRs over a certain size? If so, enable it. Additionally, you need to beat management over the head with the certain knowledge that PRs like this will lead to a company wide crisis. Data theft and loss, runaway costs, and major instability/unavailability are guaranteed with this process. Even if they don’t listen to you now, you will at least be known as the person who rang the alarm bells before the crisis.
ugh_my_@reddit
Of course it is
livando1@reddit
Bring up reviewer fatigue at retro. Work with AI to research alternatives.
likeittight_@reddit
How does this have upvotes lol
Come on people
dreamingwell@reddit
You’re worrying about the wrong thing. Big PRs doesn’t equal bad. Bad PRs are bad.
Focus on end to end test coverage that runs on every build. Reject builds that fail.
Use AI to validate that tests are correct, and that they aren’t changed inappropriately.
You’ll be much more happy.
throwaway_0x90@reddit
Are you in a position to bring about change? If you're not senior/lead/manager/etc then all you can do is suggest.
No matter what though, you can only bring about change if you can prove a measurable positive impact.
Exactly what problems are these huge PRs causing and how would making them smaller solve that problem? And are these problems
*measurable*and important to the business.psyyduck@reddit
You have to also look at the future. LLMs can scale to outputting millions of LOC per minute. Human attn simply can't scale do that. You either get rid of human input, or you severely restrict the bot output (enforce this with automated scanners).
F0tNMC@reddit
I think the best that you can do is be the change you want to see. I think with the new AI capabilities it might be possible to build robust software using large PR’s, but I don’t think it’s very probable. Unless they are very strong, conventions and standards each PR will diverge from the standards and the design will become more fragmented.
So I would do my best to follow your own standards and design conventions within whatever corner you can control. And resist as much as you can the temptation to go along with the herd. It’s a brave new world out there and I suspect that coding will look as different two years from now as it is different now from two years ago. Good luck to us all!
jaymangan@reddit
How often is this causing merge conflicts? I think that and review time & efficacy are some of the larger drawbacks.
All of the above is simplified if the team uses feature flags to gate new functionality/slices, and then those smaller PRs into a feature branch can instead be smaller PRs straight to the trunk (with the feature behind the flag).
Flags also have the added benefit of easily disabling features if there is an unexpected consequence, like missing an edge-case for an older data format still permitted in the DB for legacy users, etc.
cmpthepirate@reddit
This is poor design.
zurribulle@reddit
"This is too big to review efficiently, as we agreed on X meeting it'd be better to split it. Could you please create separate PRs for Y and Z functionalities/refactor?"
MoreRespectForQA@reddit
In these situations I would try suggesting autorejecting PRs over a certain size.
If everyone actually wants smaller units of work they should be into it.
If not you'll find out where the issue is by raising the idea.
Id also question if CI pipelines take too long to run or if PRs take too long to get reviewed. You may need to fix these things at the same time.
mx_code@reddit
They hand-off their project delivery to AI -> you hand off your project review to AI
My only comment is that this is not limited to your team, it's widespread accross the industry and in similar fashion to situations like this in the past we are still wrapping our head around AI is reshaping how we build software.
My only suggestion is not to overthinking, pass the reviews through an AI agent. Over time, this process will mature and the results will trickle down to engineering teams