7 ms·

Stop sending me huge PRs; a rant

149 points by trezm 2mo ago

ventana 2mo ago
Just an idea which I haven't personally tried: AI agents understand technical limitations, such as CI failures. Maybe make a CI job which checks that a PR has a reasonable size, and auto-reject with a polite message if it's not? Something like, "This PR size exceeds the limit of N lines that we accept for review; if you implement a big feature please consider splitting it in several smaller PRs." There are chances that it won't help, but it might!
- striking 2mo ago
  Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.
  - t-writescode 2mo ago
    How? If one PR builds off another, won’t either: PR 1 is size 400 PR 2 is size 400 + 400 new PR 3 is size 800 + 400 new If they’re truly disjoint, would it be so bad to get them as unique? Because otherwise, when PRs depend on each other, you tend to get “one and then one and then one”. At least that’s how it’s worked on teams I’ve worked on that have soft size limits.
    - eek2121 2mo ago
      Size is an issue, but it isn't just about size. Ideally, agile development builds linearly in complexity. Rather than dumping a huge new feature, first introduce the building blocks and the reason you are introducing them, then the glue that ties them together, then the actual feature. From what I've seen (not in software dev anymore, however I've been in it for close to 30 years), AI just tends to pile everything in, and it is very hard to review. No public model performs even average under the rules I've mentioned. Also, simply breaking up a PR doesn't count if instead you dump all the PRs on maintainers at once. Humans are the bottleneck here, and can only review so much at once. If i were still involved in PR reviews, it doesn't matter if you gave me a single 4,000 line PR or 4 1,000 line PRs, I"d reject them. What I want to see. Small, easily reviewable features with a build up to the main course, along with a good explanation for each. After that? I'd probably still reject it for a breach of code standards, or documentation, or because I don't like you sending me a PR at 4:59pm on a Friday. ;) Humans also can't blindly rely on AI for review, so the models (more precisely, the folks building the underlying stuff) must adapt.
  - wiml 2mo ago
    Then reject them for being incomprehensible? Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
    - striking 2mo ago
      I do think code review is worthwhile, not sure how you read that from my comment. A cap on PR size isn't inherently going to make an LLM do a good job of segmenting PRs. It requires careful prompting or manual action, the kind of effort typically exerted by people who already cared enough not to hit such a cap. You may as well just ditch the cap, to save yourself from having to reject a series of PRs rather than just the one.
      - paimapi 2mo ago
        is there a reason why there's no standardization in orgs in terms of skills/harnesses/etc for AI-assisted development? for example, a rule of 'you must invoke ABC skill that contains all of the context for this part of the codebase if you plan on making changes there' with the codeowning team dedicated to maintaining it both for their own use and for the use of other teams that have up or downstream dependencies
      - Arainach 2mo ago
        This is defeatism. Reject unacceptable PRs, full stop. It's on the author to work to break them up.
  - IshKebab 2mo ago
    Luckily GitHub supports stacked PRs now! So they have to at least pass CI individually.
- ok_computer 2mo ago
  That message could also be generated by a PR line count rule and string replacement without the ai overhead. Edit - apologies I misunderstood which side the ai agent should be on.
  - throwaway2037 2mo ago
    You raise a good point here. Why do we need a CI job? Just tell the AI/LLM: The max number of lines per PR is X. If you need more lines, please create a chain of PRs. That should work OK.
esafak 2mo ago
A simple solution is to use a git hook that asks for confirmation if it is too big, with a suggestion to ask the user to have the agent split it up. For OSS, my suggestion is to accept issues and specs do the implementation yourself. Warp.dev has a decent model of this in Github: https://github.com/warpdotdev/warp/blob/master/CONTRIBUTING.md https://github.com/warpdotdev/warp/blob/master/CONTRIBUTING....
- hahahaa 2mo ago
  Yes could be a pipe failure. Commonly used for coverage or security concerns, it could be also used for PR size.
stackskipton 2mo ago
I know someone working on a smaller open source who has same thing. They have considered just blocking all PRs outside known contributors because AI spam even on their tiny open source project is too much. At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
- throwatdem12311 2mo ago
  CI by itself is not got enough because LLMs are extremely good at writing vacuous tests that don’t actually test anything but look like the test something. Even worse: they can write tests that make incorrect behavior part of your spec. Tests matter. Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application code anymore, you should at least be going over the tests with a fine toothed comb.
  - stackskipton 2mo ago
    It's all we got at this point. Even as SRE, I just got 2000-line Golang change to something I think should be 150. However, the boss is already bouncing around happy we are going to deliver something that's been in Jira backlog for 9 months.
    - throwatdem12311 2mo ago
      Why even bother then? Just feed Jira tickets into Claude Code and have it write the code, open the PRs have Claude in a GitHub action that does a code review on PRs, a routine that resolves the reviews, rebases the code and fixes conflicts and finally another that just merges anything that’s green in CI, no outstanding review and no conflicts. Then just spin in your chair whistling all day I guess. Surely your boss will be ecstatic.
      - stackskipton 2mo ago
        Pretty sure that's what a group in my company is working on now. Except, I won't be spinning in my chair, I'll be out of a job. At least until cost skyrockets and outages get much worse.
gensym 2mo ago
> why did you put it up for a human review at all then? This seems to be the crux of the issue. I'm guessing the most of the time, the answer is "because that's a mandatory gate to getting these changes into production". If the PR author doesn't see the value in review, it's going to be hard to convince them to write reviewable PRs. If they're actually looking for human feedback, telling them how to submit PRs in a way that's amenable to human feedback is going to be a lot more successful.
- skydhash 2mo ago
  Pretty much this. In OSS, review is mostly about convincing the others that your change is good and useful enough to merge. In corporate, it's seen as a blocker to change the ticket status to done. The vibe of the latter is mostly "it's working on my computer, approve it so that we can reach the quota for the sprint".
- lokar 2mo ago
  I would back up. If leadership is not committed to real reviews, it’s not your job to make them happen. Don’t try to fight an impossible fight no one cares about. Personally, I would leave. But that’s not always an option for everyone.
danpalmer 2mo ago
In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle, that's even more. I don't think any of this scales linearly in the amount of code or number of commits. This is all in addition to the fact that the models are generally poor at storytelling, because that requires a theory of mind of the person you're communicating with. Authoring for review is storytelling, it's making changes in such a way as to build confidence in the reviewer. I believe current LLMs are still years away from this. In my opinion, if you can't do these things, you're just cosplaying software engineering. Vibe coding has its uses, as does LLM programming, I do a lot of this! But we're kidding ourselves and dropping our standards dangerously low if we think that this is software engineering.
- t-writescode 2mo ago
  I mean, sometimes I don’t know how I want to write something until I’m finished. Huge refactors are often like this. So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context. It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing this back to you and you’re asked to review it.
  - mattm 2mo ago
    > sometimes I don’t know how I want to write something until I’m finished This is knowledge that goes back to the beginning of software development - "Plan to throw [version] one away". I think this could potentially become a good practice. LLMs make it so easy and cheap to just get it working and build that v1. Then you can play around with it and see if works and read the code about what could be better. Throw away the LLM generated version and now this is the part where human expertise comes in. Based on what you've learned from the v1, now guide the LLM more closely about how to write the thing and help guide it so that making small PRs that are easily reviewable and understandable are the output.
usewik 2mo ago
> If your variable is not named well and you need a comment, name your variable better. 100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.
- t-writescode 2mo ago
  Indeed. If you’re going to have an essay on top of a function or anywhere in code, earn the essay. That code better be operating on a ton of assumptions or using some creative logic to get to how it is that a simple reading doesn’t make sense. I’ve done it myself on: * engine definitions for complex workflows and DSLs * heavy graph theory sections that included ASCII diagrams to clarify flow. But those functions are probably 1 in 100 or rarer. Basically everything else is good enough with basic IDE-helping javadoc style comments at best, maybe with some input parameter clarification and business logic-clarifying 1-2 line comments sprinkled throughout.
- throwaway2037 2mo ago
  > Clean Code [U]ncle Bob style Without starting a flame war, in 2026, is this still really a thing? I cannot recall any developer that I thought was excellent ever quoting "Clean Code [U]ncle Bob style" as gospel for how to write your code. There are just so many silly rules that he touts.
  - usr1106 2mo ago
    Has it ever been a thing in most companies? I have seen definitely more bad code and negligance than serious, skillful attempts to write clean code. Which rules do you think are particularly outdated?
  - lubujackson 2mo ago
    Like all extremists, he takes a mild view "if you have to explain the code, maybe it needs to be refactored" to "no comments ever". Never mind business logic or footguns that can't be fixed right now because of Reasons or "I tried to change this and it blew up because of some remote script calls it that isn't in the codebase" or any number of other very useful comments that inform engineers (and now LLMs) as guideposts along the way. But I worked at a company that loved Uncle Bob and enforced "no comments!" as a rule... just guess how clean and tidy that codebase was!
bawolff 2mo ago
>I'm tired boss. I'm tired of reviewing one, two, three thousand line PRs because some agent was able to "one shot the whole issue." Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer. 100% but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.
- sipjca 2mo ago
  As a maintainer just saying no and closing PRs is largely the solution
  - Tadpole9181 2mo ago
    Sure, for FOSS. But at a business I'm not sure you'll stay employed long if you do this regularly and don't try to find ways to adapt to the new world we live in. To be clear, I don't have an answer either. Reviews have definitely become a bottleneck, and the agent-created code has absolutely not reached a "broadly trustworthy" state for complex problems and maintainable contributions.
    - sipjca 2mo ago
      Agree, I was not talking about a business context. But if people are submitting garbage in a business context you got other problems you need to address.
    - Anamon 2mo ago
      Depends on the corporate environment. Refusing to look at oversized PRs makes good business sense, there's an argument to be made that working like this sabotages the quality of the product.
feelamee 2mo ago
> Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer I think they were asked before AI and even they were not easier to write. Its same as with commits. Usually when implementing a new feature I'm just in flow, so I don't think how to properly separate changes to different commits. I mean - not always, but usually maintaining git history in a beautiful and clean manner was extra work even before AI.
- esikich 2mo ago
  Yeah the one and only time I attempted an open source PR, it was for a performance improvement for one small part of the software but touched a zillion files. After looking at the PR I decided not to submit it because it just looked like a mess and I didn't really know how to split that sort of thing up at the time. AI might make this sort of thing more common, but it's certainly not new.
- skydhash 2mo ago
  Then you may need to improve your git-fu (or $vcs-fu). I use magit, so it's always easier to select only the lines/hunks/files that is for one specific change, stage and commit that. Before magit, I use sublime merge, Intellij vcs feature, and fugitive. My flow state is for editing files. Once that's done and I've got something that work. It's always easy to convert those into sensible commits. Do not that the logs is not the like of "write database schema * add the index page * add the details page * add the new object form". They're more like "show the list of objects * allow object creation * show the details of a specific object". Those breaks to create the commits are more natural to the general flow state.
thunkle 2mo ago
There's only one way out of this predicament. AI reviews. It's what we have to do.
- the_sleaze_ 2mo ago
  Who audits the reviewer?
- ovao 2mo ago
  Adversarial review is a thing, but it’s not the thing. Slop begets slop.
mojojojo_dev 2mo ago
Stumbled upon this the other day via my LinkedIn and now sanely look the jst code https://chromewebstore.google.com/detail/github-pr-focus/pebcoihjeobifgbigebbnbjdmmagpgga https://chromewebstore.google.com/detail/github-pr-focus/peb... But the first thing I still check is consecutive comments and that goes very far as a signal whether the person sending it even tried to grok it or not
mhh__ 2mo ago
I really believe people who publish huge slop PRs (short of being fired) should have their tokens taxed on the basis that it's an unpriced cost on the colleagues and the firm
- ls-a 2mo ago
  [flagged]
lokar 2mo ago
If the whole thing is really all or nothing (very rare), at least break it up into sensible commits that can be reviewed individually.
- singpolyma3 2mo ago
  If it can be broken into commits it's not all or nothing is it :)
  - lokar 2mo ago
    You could allow commits in the same PR that must all go together, and then squash them before you merge
dpc94 2mo ago
while we are at it, stop filling in the PR body with a mini novella of text generated by ai. they are hard to review and are unnecessarily verbose. the description should be there to benefit the reviewer.
dboreham 2mo ago
I worked for a human for a while who complained the same way. Problem was: it was a religion for him, not based in any reasonable logic. The large PRs needed to be large because they were adding features that couldn't be half-pregnant. The feature needed to be implemented fully in order to demo to customers or management. Once you have the whole thing coded and working it makes no sense to artificially split it into smaller pieces. That's unnecessary work you're doing only to satisfy the bloke with the beef about large PRs. Anyway, absolutely none of that had anything to do with LLMs -- it was a function of a person who liked to control other people as much as possible. With LLMs I find they positively like to attack problems in small pieces. I can't recall ever having to ask one to subdivide the work. They usually just do that anyway.
- throwaway2037 2mo ago
  > The large PRs needed to be large because they were adding features that couldn't be half-pregnant. I'm not convinced here. I have worked on a wide variety of large, complex software systems throughout my long career. Never once could we not stage a large new feature using multiple PRs and feature flags. And before you pushback, remember that Google Chromium, which is objectively one of the largest and most complex open source projects in history, makes extensive use of this strategy for rolling out large features. See: chrome://flags/ "If there is a will, there is a way."
deleted 2mo ago
[deleted]
4lx87 2mo ago
So ask the LLM to split it up into PRs of your preferred size. Or better yet, stop reviewing the code and review the working software instead. LLMs give far more substantive code reviews than humans and have for a while now.
leeoniya 2mo ago
It's possible to do a large amount of AI-assisted work, then do a second/third/fourth pass to break it up into a reviewable stack of self-contained PRs. But it takes time, and there's no such thing as one-shotting it. And it's basically impossible to continuously rebase manually without burning tokens. The way to merge the stack is more-or-less "stop the world". However, I have yet to see how this will play out with upstream contribs: https://github.com/moment/luxon/discussions/1796 https://github.com/moment/luxon/discussions/1796 https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches/benchmarks/patches https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches...
jobuildsstuff 2mo ago
[flagged]
biglyburrito 2mo ago
"Yes, you're right to push back on that."
- bot403 2mo ago
  The decision of how to split this PR is genuinely yours.
willswire 2mo ago
Paginate atomic commit diffs
orangecat 2mo ago
Apparently I'm in the minority, but if a single chunk of functionality legitimately needs a thousand lines of code, I'd rather see it all up front than have to review 5 separate PRs that don't do anything until they're combined.
- tmtvl 2mo ago
  If a massive PR can't be broken up into multiple small but meaningful sub-PRs then it's normally good manners to say something like 'here's what I want to change, this is why it needs changing, and I think it needs this massive overhaul which touches these million and one things'. But I don't think that's a likely scenario unless a codebase is poorly designed and the prevailing wisdom of 'to make a difficult change: first make the change easy, then make the easy change' will usually be applicable.
- mulmboy 2mo ago
  A good middle ground is to have a large PR open for reference, and then split sections of it out into separate PRs. This way you get nice reviewable chunks while also having the broader context. This is similar to stacked PRs except that it's much easier to tweak things as you split them out without rebase shenanigans although of course if you tweak too much it kind of defeats the point of the whole thing. A nice thing about this is that you can put the large PR up while it's still very draft for conceptual review (socialisatuon etc etc) while you work on splitting out and polishing parts of it.
markbao 2mo ago
Y'all need to try PR review tools that split PRs into chapters. Stage was the best product in this space, but Linear Review has it, Graphite has it, and some other tools too. You get the full contexts while each piece is still reviewable individually.
sackfield 2mo ago
It is reasonable to break PRs up into smaller chunks, but there is a limit. There are frequently reviewers who get very zealous about this and insist on breaking things up beyond what is reasonable, for example if breaking it up would destroy the intent, or if the "thousand line" PR just contains lots of tests (AI's love to write tests, and I love that they do that). Some tasks are just long, and its important to contextualise this when reviewing. In the end though, these reviewers will die off like the dinosaurs. The article actually states that they find the idea of reviewing a large PR with AI bad because "it wastes your tokens reviewing a reingesting code that was already made by an AI". This doesn't make a whole lot of sense, AI will frequently reingest AI generated content, evals are a great example of this. Just after this the article touches on the real issue at play: "okay, great, why did you put it up for a human review at all then?". Indeed, this is a good question to ask, why do we put it up for human review? I would wager that they don't actually want human feedback, a human has placed themselves as a gatekeeper and thus must be placated, and probably chooses the most inefficient way to keep that gate slowing everyone down who has kept up with the technology of our times.
- throwaway2037 2mo ago
  I like this comment. I also have a lot of experience with lazy (my accusation!) reviewers who ask me to split a PR into smaller PRs. At some jobs, I felt like it was a strategy to sabotage my work (slow me down). In my experience, the best was to ensure your code will be approved (ok, maybe some minor tweaks) is to assign a code reviewer before you start writing code. You (the coder) performs some quick analysis, then formulates a plan for how to fix the bug or implement a change or new feature. You verbally discuss this plan using screenshare (or side-by-side in-person) so both of you can see the code that will be changed. The code reviewer needs to verbally approve your plan. This way, you don't spend a lot of time writing code and tests, only to have the reviewer rejected it very quickly: "You should have done it this way instead." (Please note: That process that I described is intentionally informal, casual, and unrigid. Why? This grants permission for the coder and reviewer to decide the plan as intelligent adults, not as "Children of JIRA" [hint: unnecessary formality].) Another thing that I do: After I write the code, but before I write tests, I ask the reviewer to review, but not approve. After looking at their comments, I quickly add another commit to the PR to address their concerns, then start work on the tests. When the reviewer does the final review, it is very quick, like 10% of the initial review. All of this really helps to reduce coder/reviewer friction, and nearly eliminate "Tyranny of the Reviewer". One last gripe about code reviewing: It hardly makes sense for someone much more junior than the coder to do the review. I have seen this too many times.
sqemo 2mo ago
Isn’t having humans review every PR only realistic for smaller systems? If AI is generating all these PRs, but humans still have to review every one of them, doesn’t that ultimately leave the humans responsible for everything?
- TheChaplain 2mo ago
  Someone has to be responsible, because it sure won't do to call Anthropic at 4 am and ask why Claude caused your service to rack up $5m on AWS and delete all backups.
dlevine 2mo ago
I think this is more a symptom of the problem than the actual problem. The issue is that we can generate tons of code using AI, but then are blocked on having humans review all of it. I don’t think we should auto-approve all of this code without human review - that clearly doesn’t work either. What I do think we need is probably at least two-fold 1) better ways to explain these big PRs to human reviewers. 2) better ways to verify the functionality of a piece of code. Things like auto generating walkthrough videos I’m not sure that even this is enough. I’m sure there will be agents that try to solve this problem.
- bigstrat2003 2mo ago
  Or people could stop using LLMs to generate huge swaths of code, and companies could discipline those who refuse to stop. It's providing negative value at this point.
- Arainach 2mo ago
  > Things like auto generating walkthrough videos I'm trying to write this with respect, but please explain your thought process here. If your PR description is a video instead of a written explanation, I'm rejecting it without even reading the code.
  - dlevine 2mo ago
    It’s not an either/or. It’s doing both. The video makes it easier to understand what the code does. We have used Looms on PRs since before AI, and agents are beginning to be able to do this on PRs they generate.
- ArnaudDebray 1mo ago
  Agree with you. Going even further, I'm wondering whether the Diff alone is still the right review artifact... On your 1), I see Entire.io is trying to build some useful building blocks (capturing all agent sessions and prompts to extract the human intent). With a friend, we're currently working on a tool to ease the review using entire's agent session
ulrikrasmussen 2mo ago
If you generate PRs too big to review for others, then they are too big to review for yourself. This means you are delegating the task of understanding the code to an LLM, and the end result is inevitably that noone in the organization understands the code better than someone who just walked in the door. They can write the next LLM prompt just as well as you because they know as little about the system as you. In that situation I ask you: what is your moat as a software company? Why would your customers keep paying you when companies like Anthropic can just do your-software-company-as-a-service and cut out the middle man and six figure salaries?
oliwarner 2mo ago
I'm surprises GitHub haven't added a mechanism to donate tokens to let a project maintainer inspect, verify, explain and test PRs. For big PRs it would make a difference if I didn't have to check everything for malicious links and junk without spending my own money or time.
moezd 2mo ago
There's also another approach: Not all PRs should introduce new, functioning features or complete rewrites. Thus you can just introduce a handful of functions, some of the functionality behind a feature flag, a new db schema... and then introduce the rest in follow-up PRs. GitHub even does stacked PRs for this reason, so you can start big, organize your commits into these chunks and do the stacked PRs. Having AI generate massive code and shoving it to your teammates' plates should be considered irresponsible. Yes, test code is verbose, but it should be accompanied with an adequate description of what is currently tested. Otherwise it's just AI having fun in your codebase. Automated PR merges could still work, please just don't with multiple thousands of LoC changes. Both LLMs and humans have a context size limit.
oxedom 2mo ago
I recently started stacking my PRs, makes life easier https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests https://docs.github.com/en/pull-requests/how-tos/stacked-pul...
- forbidden404 2mo ago
  LLMs will still produce huge PRs even in a stack if you are not careful enough.
  - oxedom 1mo ago
    That's a different problem. The benefit of stacked PRs is that they let you review each piece on its own, which helps you/team keep mental alignment with changes in the codebase and disregard individual slop layers.
ianmarcinkowski 2mo ago
We were forced to merge 2 HUGE PRs this week because of a customer deadline. It didn't need to be this way, but our AI maximalist team member went full toxenmaxxing and we had 92 frontend files and 40ish backend. Untestable, touched nearly the entire application stack, had several trivialities that were huge sticking points in review that obscured other more substantive issues we should have spent review time on.
albatross79 2mo ago
The whole point of using AI is to generate more code more quickly. If you cant keep up, then step out of the way. Some people think being a reviewer is a privileged position. Well now you've got your work cut out for you. Ultimately the solution will be to get rid of reviews and reviewers, and put the responsibility for the code on the "author" (prompter). If you're using AI to generate code you're already mostly just a reviewer, putting more reviewers on the same code just slows everything down. What's needed is more accountability.
- kimos 2mo ago
  What does “accountability” mean to you here. Isn’t that the point of a reviewer to make them also accountable for the PR? I think what you’re outlining here is an entire change to the code creation and merging process, not just adding an AI helper to the “coder”.
  - albatross79 2mo ago
    It means the stakes are higher for individuals. The coding task is easier because of ai, but because fewer people are going to review it you have more responsibility for it. You can't push out slop that "should work" or "works on my machine", etc. and put the burden on reviewers to catch edge cases. You own the result and what follows so you need to think ahead more. The coding is easier, the consequences are more personal.
hbogert 2mo ago
i felt this. My majority of time correcting claude is making it stop the stupid verbose doc comments which are completely contemporary. I can add skills/memories and claude.md hints that i want, but every time it continues with those unnecessary doc comments of 5+ lines explaining some situation which should definitely not be in a doc comment.
cadamsdotcom 2mo ago
This to me reads less as there being some objective level when a PR becomes "huge" and more about the tension between a system's ability to absorb change vs. our tools' ability to create change. By way of analogy consider the relative impact on an ecosystem of one person fishing with a fishing line vs. a commercial fishing boat trawling the ocean. Of course, one person fishing is unlikely to have a huge impact on the ocean so it's generally permitted. Trawling (agentic coding) can be done in a way that's destructive to ecosystems but it can also be done sustainably! So with the analogy in mind let's bring back the "sensible trawling" idea to agentic coding. What might it look like to solve the "huge PR bad" constraint in another way: by increasing our codebases' ability to absorb change, so what "a huge PR" is, becomes bigger? Probably needs solves at many levels: assistance quickly comprehending the PR (AI driven walkthroughs, multiple media expected from the PR submitter not just text - eg. a screencast walkthrough of it), it requires rethinking how the code is read (better review tooling); it requires integrations with code-review automation tools (both you home-grown checklist and third-party tools) it requires rigorous testing (comprehensive automated e2e; test-driven; functional tests; etc); it requires putting the actual "in the loop" so post-release fast-follows can be expedited (eg. product signals and Sentry and metric anomalies are fed back in for quick follow up releases) If you can be so much more responsive to the customer and market. Eg. you can unlaunch features just as easily as you launched them - and you can finally clean up all that tech debt. Better for the business better for the codebase and better for developer happiness. Not all these strategies work for every situation - you can't do post-release in the loop if the shit needs to work first time! But the whole idea creates so much richness in applying human judgment and engineering solutions and it's all brand new because we never needed to deal with this much change before Think of it as "releases in the loop". If opportunities to rethink the stack to support MORE change excite you, congratulations! You're ready for the future that's coming. If you don't like this - get yourself into a job where you can say no a lot, or where shit needs to work first time, and you can be happy. Test-driven, strongly reviewed.. there's ways with agentic coding to also make super high quality stuff. But you can also shoot product from the hip more accurately and more often than ever before. It won't be applied correctly everywhere - it's still heavily judgmental dependent and we're all fallible - but there'll be a much wider spectrum of options for how to build products. I think this is a really exciting future!
tartieret 2mo ago
I have definitely noticed this in our organization as people use more AI tools. At first educating by providing feedback to the developers, then after a few repeat directly requesting that a PR is broken down if there is an obvious way to do it. But the thing that really made the differences: 1) having a small github action that checks the size of a PR and leave a warning comment if it's large. Obviously some PR have to be large but then the developer has to justify 2) much better: getting early access to Github Stacked PR. we all like the experience and it solves a number of problems. without it even if you are disciplined and break down your work on several PRs, you end up having to deal with rebasing them one by one when the base moves. We modified Claude.md so that it tries to use it when it makes sense and now even AI generated changes result in stack of small PRs I think it's worth investing in this as there are studies showing that review time is exponential with the size of PRs (or worse you are more likely to let defect go through on a large PRs). And AI agents are also better are reviewing smaller chunks. I had personally several experiences of asking someone to break down a super large PR into smaller ones and found a defect in PR#2 which wasn't caught in the original AI driven PR review

149 points by trezm 2mo ago

Just an idea which I haven't personally tried: AI agents understand technical limitations, such as CI failures. Maybe make a CI job which checks that a PR has a reasonable size, and auto-reject with a polite message if it's not? Something like, "This PR size exceeds the limit of N lines that we accept for review; if you implement a big feature please consider splitting it in several smaller PRs." There are chances that it won't help, but it might!

striking 2mo ago
Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.
- t-writescode 2mo ago
  How? If one PR builds off another, won’t either: PR 1 is size 400 PR 2 is size 400 + 400 new PR 3 is size 800 + 400 new If they’re truly disjoint, would it be so bad to get them as unique? Because otherwise, when PRs depend on each other, you tend to get “one and then one and then one”. At least that’s how it’s worked on teams I’ve worked on that have soft size limits.
  - eek2121 2mo ago
    Size is an issue, but it isn't just about size. Ideally, agile development builds linearly in complexity. Rather than dumping a huge new feature, first introduce the building blocks and the reason you are introducing them, then the glue that ties them together, then the actual feature. From what I've seen (not in software dev anymore, however I've been in it for close to 30 years), AI just tends to pile everything in, and it is very hard to review. No public model performs even average under the rules I've mentioned. Also, simply breaking up a PR doesn't count if instead you dump all the PRs on maintainers at once. Humans are the bottleneck here, and can only review so much at once. If i were still involved in PR reviews, it doesn't matter if you gave me a single 4,000 line PR or 4 1,000 line PRs, I"d reject them. What I want to see. Small, easily reviewable features with a build up to the main course, along with a good explanation for each. After that? I'd probably still reject it for a breach of code standards, or documentation, or because I don't like you sending me a PR at 4:59pm on a Friday. ;) Humans also can't blindly rely on AI for review, so the models (more precisely, the folks building the underlying stuff) must adapt.
- wiml 2mo ago
  Then reject them for being incomprehensible? Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
  - striking 2mo ago
    I do think code review is worthwhile, not sure how you read that from my comment. A cap on PR size isn't inherently going to make an LLM do a good job of segmenting PRs. It requires careful prompting or manual action, the kind of effort typically exerted by people who already cared enough not to hit such a cap. You may as well just ditch the cap, to save yourself from having to reject a series of PRs rather than just the one.
    - paimapi 2mo ago
      is there a reason why there's no standardization in orgs in terms of skills/harnesses/etc for AI-assisted development? for example, a rule of 'you must invoke ABC skill that contains all of the context for this part of the codebase if you plan on making changes there' with the codeowning team dedicated to maintaining it both for their own use and for the use of other teams that have up or downstream dependencies
    - Arainach 2mo ago
      This is defeatism. Reject unacceptable PRs, full stop. It's on the author to work to break them up.
- IshKebab 2mo ago
  Luckily GitHub supports stacked PRs now! So they have to at least pass CI individually.
ok_computer 2mo ago
That message could also be generated by a PR line count rule and string replacement without the ai overhead. Edit - apologies I misunderstood which side the ai agent should be on.
- throwaway2037 2mo ago
  You raise a good point here. Why do we need a CI job? Just tell the AI/LLM: The max number of lines per PR is X. If you need more lines, please create a chain of PRs. That should work OK.

esafak 2mo ago

A simple solution is to use a git hook that asks for confirmation if it is too big, with a suggestion to ask the user to have the agent split it up. For OSS, my suggestion is to accept issues and specs do the implementation yourself. Warp.dev has a decent model of this in Github: https://github.com/warpdotdev/warp/blob/master/CONTRIBUTING.md https://github.com/warpdotdev/warp/blob/master/CONTRIBUTING....

hahahaa 2mo ago
Yes could be a pipe failure. Commonly used for coverage or security concerns, it could be also used for PR size.

stackskipton 2mo ago

I know someone working on a smaller open source who has same thing. They have considered just blocking all PRs outside known contributors because AI spam even on their tiny open source project is too much. At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.

throwatdem12311 2mo ago
CI by itself is not got enough because LLMs are extremely good at writing vacuous tests that don’t actually test anything but look like the test something. Even worse: they can write tests that make incorrect behavior part of your spec. Tests matter. Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application code anymore, you should at least be going over the tests with a fine toothed comb.
- stackskipton 2mo ago
  It's all we got at this point. Even as SRE, I just got 2000-line Golang change to something I think should be 150. However, the boss is already bouncing around happy we are going to deliver something that's been in Jira backlog for 9 months.
  - throwatdem12311 2mo ago
    Why even bother then? Just feed Jira tickets into Claude Code and have it write the code, open the PRs have Claude in a GitHub action that does a code review on PRs, a routine that resolves the reviews, rebases the code and fixes conflicts and finally another that just merges anything that’s green in CI, no outstanding review and no conflicts. Then just spin in your chair whistling all day I guess. Surely your boss will be ecstatic.
    - stackskipton 2mo ago
      Pretty sure that's what a group in my company is working on now. Except, I won't be spinning in my chair, I'll be out of a job. At least until cost skyrockets and outages get much worse.

gensym 2mo ago

> why did you put it up for a human review at all then? This seems to be the crux of the issue. I'm guessing the most of the time, the answer is "because that's a mandatory gate to getting these changes into production". If the PR author doesn't see the value in review, it's going to be hard to convince them to write reviewable PRs. If they're actually looking for human feedback, telling them how to submit PRs in a way that's amenable to human feedback is going to be a lot more successful.

skydhash 2mo ago
Pretty much this. In OSS, review is mostly about convincing the others that your change is good and useful enough to merge. In corporate, it's seen as a blocker to change the ticket status to done. The vibe of the latter is mostly "it's working on my computer, approve it so that we can reach the quota for the sprint".
lokar 2mo ago
I would back up. If leadership is not committed to real reviews, it’s not your job to make them happen. Don’t try to fight an impossible fight no one cares about. Personally, I would leave. But that’s not always an option for everyone.

danpalmer 2mo ago

In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle, that's even more. I don't think any of this scales linearly in the amount of code or number of commits. This is all in addition to the fact that the models are generally poor at storytelling, because that requires a theory of mind of the person you're communicating with. Authoring for review is storytelling, it's making changes in such a way as to build confidence in the reviewer. I believe current LLMs are still years away from this. In my opinion, if you can't do these things, you're just cosplaying software engineering. Vibe coding has its uses, as does LLM programming, I do a lot of this! But we're kidding ourselves and dropping our standards dangerously low if we think that this is software engineering.

t-writescode 2mo ago
I mean, sometimes I don’t know how I want to write something until I’m finished. Huge refactors are often like this. So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context. It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing this back to you and you’re asked to review it.
- mattm 2mo ago
  > sometimes I don’t know how I want to write something until I’m finished This is knowledge that goes back to the beginning of software development - "Plan to throw [version] one away". I think this could potentially become a good practice. LLMs make it so easy and cheap to just get it working and build that v1. Then you can play around with it and see if works and read the code about what could be better. Throw away the LLM generated version and now this is the part where human expertise comes in. Based on what you've learned from the v1, now guide the LLM more closely about how to write the thing and help guide it so that making small PRs that are easily reviewable and understandable are the output.

usewik 2mo ago

> If your variable is not named well and you need a comment, name your variable better. 100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.

t-writescode 2mo ago
Indeed. If you’re going to have an essay on top of a function or anywhere in code, earn the essay. That code better be operating on a ton of assumptions or using some creative logic to get to how it is that a simple reading doesn’t make sense. I’ve done it myself on: * engine definitions for complex workflows and DSLs * heavy graph theory sections that included ASCII diagrams to clarify flow. But those functions are probably 1 in 100 or rarer. Basically everything else is good enough with basic IDE-helping javadoc style comments at best, maybe with some input parameter clarification and business logic-clarifying 1-2 line comments sprinkled throughout.
throwaway2037 2mo ago
> Clean Code [U]ncle Bob style Without starting a flame war, in 2026, is this still really a thing? I cannot recall any developer that I thought was excellent ever quoting "Clean Code [U]ncle Bob style" as gospel for how to write your code. There are just so many silly rules that he touts.
- usr1106 2mo ago
  Has it ever been a thing in most companies? I have seen definitely more bad code and negligance than serious, skillful attempts to write clean code. Which rules do you think are particularly outdated?
- lubujackson 2mo ago
  Like all extremists, he takes a mild view "if you have to explain the code, maybe it needs to be refactored" to "no comments ever". Never mind business logic or footguns that can't be fixed right now because of Reasons or "I tried to change this and it blew up because of some remote script calls it that isn't in the codebase" or any number of other very useful comments that inform engineers (and now LLMs) as guideposts along the way. But I worked at a company that loved Uncle Bob and enforced "no comments!" as a rule... just guess how clean and tidy that codebase was!

bawolff 2mo ago

>I'm tired boss. I'm tired of reviewing one, two, three thousand line PRs because some agent was able to "one shot the whole issue." Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer. 100% but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.

sipjca 2mo ago
As a maintainer just saying no and closing PRs is largely the solution
- Tadpole9181 2mo ago
  Sure, for FOSS. But at a business I'm not sure you'll stay employed long if you do this regularly and don't try to find ways to adapt to the new world we live in. To be clear, I don't have an answer either. Reviews have definitely become a bottleneck, and the agent-created code has absolutely not reached a "broadly trustworthy" state for complex problems and maintainable contributions.
  - sipjca 2mo ago
    Agree, I was not talking about a business context. But if people are submitting garbage in a business context you got other problems you need to address.
  - Anamon 2mo ago
    Depends on the corporate environment. Refusing to look at oversized PRs makes good business sense, there's an argument to be made that working like this sabotages the quality of the product.

feelamee 2mo ago

> Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer I think they were asked before AI and even they were not easier to write. Its same as with commits. Usually when implementing a new feature I'm just in flow, so I don't think how to properly separate changes to different commits. I mean - not always, but usually maintaining git history in a beautiful and clean manner was extra work even before AI.

esikich 2mo ago
Yeah the one and only time I attempted an open source PR, it was for a performance improvement for one small part of the software but touched a zillion files. After looking at the PR I decided not to submit it because it just looked like a mess and I didn't really know how to split that sort of thing up at the time. AI might make this sort of thing more common, but it's certainly not new.
skydhash 2mo ago
Then you may need to improve your git-fu (or $vcs-fu). I use magit, so it's always easier to select only the lines/hunks/files that is for one specific change, stage and commit that. Before magit, I use sublime merge, Intellij vcs feature, and fugitive. My flow state is for editing files. Once that's done and I've got something that work. It's always easy to convert those into sensible commits. Do not that the logs is not the like of "write database schema * add the index page * add the details page * add the new object form". They're more like "show the list of objects * allow object creation * show the details of a specific object". Those breaks to create the commits are more natural to the general flow state.

thunkle 2mo ago

There's only one way out of this predicament. AI reviews. It's what we have to do.

the_sleaze_ 2mo ago
Who audits the reviewer?
ovao 2mo ago
Adversarial review is a thing, but it’s not the thing. Slop begets slop.

mojojojo_dev 2mo ago

Stumbled upon this the other day via my LinkedIn and now sanely look the jst code https://chromewebstore.google.com/detail/github-pr-focus/pebcoihjeobifgbigebbnbjdmmagpgga https://chromewebstore.google.com/detail/github-pr-focus/peb... But the first thing I still check is consecutive comments and that goes very far as a signal whether the person sending it even tried to grok it or not

mhh__ 2mo ago

I really believe people who publish huge slop PRs (short of being fired) should have their tokens taxed on the basis that it's an unpriced cost on the colleagues and the firm

ls-a 2mo ago
[flagged]

lokar 2mo ago

If the whole thing is really all or nothing (very rare), at least break it up into sensible commits that can be reviewed individually.

singpolyma3 2mo ago
If it can be broken into commits it's not all or nothing is it :)
- lokar 2mo ago
  You could allow commits in the same PR that must all go together, and then squash them before you merge

dpc94 2mo ago

while we are at it, stop filling in the PR body with a mini novella of text generated by ai. they are hard to review and are unnecessarily verbose. the description should be there to benefit the reviewer.

dboreham 2mo ago

I worked for a human for a while who complained the same way. Problem was: it was a religion for him, not based in any reasonable logic. The large PRs needed to be large because they were adding features that couldn't be half-pregnant. The feature needed to be implemented fully in order to demo to customers or management. Once you have the whole thing coded and working it makes no sense to artificially split it into smaller pieces. That's unnecessary work you're doing only to satisfy the bloke with the beef about large PRs. Anyway, absolutely none of that had anything to do with LLMs -- it was a function of a person who liked to control other people as much as possible. With LLMs I find they positively like to attack problems in small pieces. I can't recall ever having to ask one to subdivide the work. They usually just do that anyway.

throwaway2037 2mo ago
> The large PRs needed to be large because they were adding features that couldn't be half-pregnant. I'm not convinced here. I have worked on a wide variety of large, complex software systems throughout my long career. Never once could we not stage a large new feature using multiple PRs and feature flags. And before you pushback, remember that Google Chromium, which is objectively one of the largest and most complex open source projects in history, makes extensive use of this strategy for rolling out large features. See: chrome://flags/ "If there is a will, there is a way."

deleted 2mo ago

[deleted]

4lx87 2mo ago

So ask the LLM to split it up into PRs of your preferred size. Or better yet, stop reviewing the code and review the working software instead. LLMs give far more substantive code reviews than humans and have for a while now.

leeoniya 2mo ago

It's possible to do a large amount of AI-assisted work, then do a second/third/fourth pass to break it up into a reviewable stack of self-contained PRs. But it takes time, and there's no such thing as one-shotting it. And it's basically impossible to continuously rebase manually without burning tokens. The way to merge the stack is more-or-less "stop the world". However, I have yet to see how this will play out with upstream contribs: https://github.com/moment/luxon/discussions/1796 https://github.com/moment/luxon/discussions/1796 https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches/benchmarks/patches https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches...

jobuildsstuff 2mo ago

[flagged]

biglyburrito 2mo ago

"Yes, you're right to push back on that."

bot403 2mo ago
The decision of how to split this PR is genuinely yours.

willswire 2mo ago

Paginate atomic commit diffs

orangecat 2mo ago

Apparently I'm in the minority, but if a single chunk of functionality legitimately needs a thousand lines of code, I'd rather see it all up front than have to review 5 separate PRs that don't do anything until they're combined.

tmtvl 2mo ago
If a massive PR can't be broken up into multiple small but meaningful sub-PRs then it's normally good manners to say something like 'here's what I want to change, this is why it needs changing, and I think it needs this massive overhaul which touches these million and one things'. But I don't think that's a likely scenario unless a codebase is poorly designed and the prevailing wisdom of 'to make a difficult change: first make the change easy, then make the easy change' will usually be applicable.
mulmboy 2mo ago
A good middle ground is to have a large PR open for reference, and then split sections of it out into separate PRs. This way you get nice reviewable chunks while also having the broader context. This is similar to stacked PRs except that it's much easier to tweak things as you split them out without rebase shenanigans although of course if you tweak too much it kind of defeats the point of the whole thing. A nice thing about this is that you can put the large PR up while it's still very draft for conceptual review (socialisatuon etc etc) while you work on splitting out and polishing parts of it.

markbao 2mo ago

Y'all need to try PR review tools that split PRs into chapters. Stage was the best product in this space, but Linear Review has it, Graphite has it, and some other tools too. You get the full contexts while each piece is still reviewable individually.

sackfield 2mo ago

It is reasonable to break PRs up into smaller chunks, but there is a limit. There are frequently reviewers who get very zealous about this and insist on breaking things up beyond what is reasonable, for example if breaking it up would destroy the intent, or if the "thousand line" PR just contains lots of tests (AI's love to write tests, and I love that they do that). Some tasks are just long, and its important to contextualise this when reviewing. In the end though, these reviewers will die off like the dinosaurs. The article actually states that they find the idea of reviewing a large PR with AI bad because "it wastes your tokens reviewing a reingesting code that was already made by an AI". This doesn't make a whole lot of sense, AI will frequently reingest AI generated content, evals are a great example of this. Just after this the article touches on the real issue at play: "okay, great, why did you put it up for a human review at all then?". Indeed, this is a good question to ask, why do we put it up for human review? I would wager that they don't actually want human feedback, a human has placed themselves as a gatekeeper and thus must be placated, and probably chooses the most inefficient way to keep that gate slowing everyone down who has kept up with the technology of our times.

throwaway2037 2mo ago
I like this comment. I also have a lot of experience with lazy (my accusation!) reviewers who ask me to split a PR into smaller PRs. At some jobs, I felt like it was a strategy to sabotage my work (slow me down). In my experience, the best was to ensure your code will be approved (ok, maybe some minor tweaks) is to assign a code reviewer before you start writing code. You (the coder) performs some quick analysis, then formulates a plan for how to fix the bug or implement a change or new feature. You verbally discuss this plan using screenshare (or side-by-side in-person) so both of you can see the code that will be changed. The code reviewer needs to verbally approve your plan. This way, you don't spend a lot of time writing code and tests, only to have the reviewer rejected it very quickly: "You should have done it this way instead." (Please note: That process that I described is intentionally informal, casual, and unrigid. Why? This grants permission for the coder and reviewer to decide the plan as intelligent adults, not as "Children of JIRA" [hint: unnecessary formality].) Another thing that I do: After I write the code, but before I write tests, I ask the reviewer to review, but not approve. After looking at their comments, I quickly add another commit to the PR to address their concerns, then start work on the tests. When the reviewer does the final review, it is very quick, like 10% of the initial review. All of this really helps to reduce coder/reviewer friction, and nearly eliminate "Tyranny of the Reviewer". One last gripe about code reviewing: It hardly makes sense for someone much more junior than the coder to do the review. I have seen this too many times.

sqemo 2mo ago

Isn’t having humans review every PR only realistic for smaller systems? If AI is generating all these PRs, but humans still have to review every one of them, doesn’t that ultimately leave the humans responsible for everything?

TheChaplain 2mo ago
Someone has to be responsible, because it sure won't do to call Anthropic at 4 am and ask why Claude caused your service to rack up $5m on AWS and delete all backups.

dlevine 2mo ago

I think this is more a symptom of the problem than the actual problem. The issue is that we can generate tons of code using AI, but then are blocked on having humans review all of it. I don’t think we should auto-approve all of this code without human review - that clearly doesn’t work either. What I do think we need is probably at least two-fold 1) better ways to explain these big PRs to human reviewers. 2) better ways to verify the functionality of a piece of code. Things like auto generating walkthrough videos I’m not sure that even this is enough. I’m sure there will be agents that try to solve this problem.

bigstrat2003 2mo ago
Or people could stop using LLMs to generate huge swaths of code, and companies could discipline those who refuse to stop. It's providing negative value at this point.
Arainach 2mo ago
> Things like auto generating walkthrough videos I'm trying to write this with respect, but please explain your thought process here. If your PR description is a video instead of a written explanation, I'm rejecting it without even reading the code.
- dlevine 2mo ago
  It’s not an either/or. It’s doing both. The video makes it easier to understand what the code does. We have used Looms on PRs since before AI, and agents are beginning to be able to do this on PRs they generate.
ArnaudDebray 1mo ago
Agree with you. Going even further, I'm wondering whether the Diff alone is still the right review artifact... On your 1), I see Entire.io is trying to build some useful building blocks (capturing all agent sessions and prompts to extract the human intent). With a friend, we're currently working on a tool to ease the review using entire's agent session

ulrikrasmussen 2mo ago

If you generate PRs too big to review for others, then they are too big to review for yourself. This means you are delegating the task of understanding the code to an LLM, and the end result is inevitably that noone in the organization understands the code better than someone who just walked in the door. They can write the next LLM prompt just as well as you because they know as little about the system as you. In that situation I ask you: what is your moat as a software company? Why would your customers keep paying you when companies like Anthropic can just do your-software-company-as-a-service and cut out the middle man and six figure salaries?

oliwarner 2mo ago

I'm surprises GitHub haven't added a mechanism to donate tokens to let a project maintainer inspect, verify, explain and test PRs. For big PRs it would make a difference if I didn't have to check everything for malicious links and junk without spending my own money or time.

moezd 2mo ago

There's also another approach: Not all PRs should introduce new, functioning features or complete rewrites. Thus you can just introduce a handful of functions, some of the functionality behind a feature flag, a new db schema... and then introduce the rest in follow-up PRs. GitHub even does stacked PRs for this reason, so you can start big, organize your commits into these chunks and do the stacked PRs. Having AI generate massive code and shoving it to your teammates' plates should be considered irresponsible. Yes, test code is verbose, but it should be accompanied with an adequate description of what is currently tested. Otherwise it's just AI having fun in your codebase. Automated PR merges could still work, please just don't with multiple thousands of LoC changes. Both LLMs and humans have a context size limit.

oxedom 2mo ago

I recently started stacking my PRs, makes life easier https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests https://docs.github.com/en/pull-requests/how-tos/stacked-pul...

forbidden404 2mo ago
LLMs will still produce huge PRs even in a stack if you are not careful enough.
- oxedom 1mo ago
  That's a different problem. The benefit of stacked PRs is that they let you review each piece on its own, which helps you/team keep mental alignment with changes in the codebase and disregard individual slop layers.

ianmarcinkowski 2mo ago

We were forced to merge 2 HUGE PRs this week because of a customer deadline. It didn't need to be this way, but our AI maximalist team member went full toxenmaxxing and we had 92 frontend files and 40ish backend. Untestable, touched nearly the entire application stack, had several trivialities that were huge sticking points in review that obscured other more substantive issues we should have spent review time on.

albatross79 2mo ago

The whole point of using AI is to generate more code more quickly. If you cant keep up, then step out of the way. Some people think being a reviewer is a privileged position. Well now you've got your work cut out for you. Ultimately the solution will be to get rid of reviews and reviewers, and put the responsibility for the code on the "author" (prompter). If you're using AI to generate code you're already mostly just a reviewer, putting more reviewers on the same code just slows everything down. What's needed is more accountability.

kimos 2mo ago
What does “accountability” mean to you here. Isn’t that the point of a reviewer to make them also accountable for the PR? I think what you’re outlining here is an entire change to the code creation and merging process, not just adding an AI helper to the “coder”.
- albatross79 2mo ago
  It means the stakes are higher for individuals. The coding task is easier because of ai, but because fewer people are going to review it you have more responsibility for it. You can't push out slop that "should work" or "works on my machine", etc. and put the burden on reviewers to catch edge cases. You own the result and what follows so you need to think ahead more. The coding is easier, the consequences are more personal.

hbogert 2mo ago

i felt this. My majority of time correcting claude is making it stop the stupid verbose doc comments which are completely contemporary. I can add skills/memories and claude.md hints that i want, but every time it continues with those unnecessary doc comments of 5+ lines explaining some situation which should definitely not be in a doc comment.

cadamsdotcom 2mo ago

This to me reads less as there being some objective level when a PR becomes "huge" and more about the tension between a system's ability to absorb change vs. our tools' ability to create change. By way of analogy consider the relative impact on an ecosystem of one person fishing with a fishing line vs. a commercial fishing boat trawling the ocean. Of course, one person fishing is unlikely to have a huge impact on the ocean so it's generally permitted. Trawling (agentic coding) can be done in a way that's destructive to ecosystems but it can also be done sustainably! So with the analogy in mind let's bring back the "sensible trawling" idea to agentic coding. What might it look like to solve the "huge PR bad" constraint in another way: by increasing our codebases' ability to absorb change, so what "a huge PR" is, becomes bigger? Probably needs solves at many levels: assistance quickly comprehending the PR (AI driven walkthroughs, multiple media expected from the PR submitter not just text - eg. a screencast walkthrough of it), it requires rethinking how the code is read (better review tooling); it requires integrations with code-review automation tools (both you home-grown checklist and third-party tools) it requires rigorous testing (comprehensive automated e2e; test-driven; functional tests; etc); it requires putting the actual "in the loop" so post-release fast-follows can be expedited (eg. product signals and Sentry and metric anomalies are fed back in for quick follow up releases) If you can be so much more responsive to the customer and market. Eg. you can unlaunch features just as easily as you launched them - and you can finally clean up all that tech debt. Better for the business better for the codebase and better for developer happiness. Not all these strategies work for every situation - you can't do post-release in the loop if the shit needs to work first time! But the whole idea creates so much richness in applying human judgment and engineering solutions and it's all brand new because we never needed to deal with this much change before Think of it as "releases in the loop". If opportunities to rethink the stack to support MORE change excite you, congratulations! You're ready for the future that's coming. If you don't like this - get yourself into a job where you can say no a lot, or where shit needs to work first time, and you can be happy. Test-driven, strongly reviewed.. there's ways with agentic coding to also make super high quality stuff. But you can also shoot product from the hip more accurately and more often than ever before. It won't be applied correctly everywhere - it's still heavily judgmental dependent and we're all fallible - but there'll be a much wider spectrum of options for how to build products. I think this is a really exciting future!

tartieret 2mo ago

I have definitely noticed this in our organization as people use more AI tools. At first educating by providing feedback to the developers, then after a few repeat directly requesting that a PR is broken down if there is an obvious way to do it. But the thing that really made the differences: 1) having a small github action that checks the size of a PR and leave a warning comment if it's large. Obviously some PR have to be large but then the developer has to justify 2) much better: getting early access to Github Stacked PR. we all like the experience and it solves a number of problems. without it even if you are disciplined and break down your work on several PRs, you end up having to deal with rebasing them one by one when the base moves. We modified Claude.md so that it tries to use it when it makes sense and now even AI generated changes result in stack of small PRs I think it's worth investing in this as there are studies showing that review time is exponential with the size of PRs (or worse you are more likely to let defect go through on a large PRs). And AI agents are also better are reviewing smaller chunks. I had personally several experiences of asking someone to break down a super large PR into smaller ones and found a defect in PR#2 which wasn't caught in the original AI driven PR review