Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Stop sending me huge PRs; a rant

getsmall.xyz

Post reply on HN
Print view View original

61–70 of 116 posts

Re: Stop sending me huge PRs; a rant

#61

Post by sackfield »

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.

Re: Stop sending me huge PRs; a rant

#62

Post by sqemo »

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?

Re: Stop sending me huge PRs; a rant

#63

Post by IshKebab »

striking wrote: post #3
ventana wrote: post #2

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 th…

Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.

Luckily GitHub supports stacked PRs now! So they have to at least pass CI individually.

Re: Stop sending me huge PRs; a rant

#64

Post by garfij »

danpalmer wrote: post #8

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,…

I have a completely different experience than you, but I have also invested quite a bit of effort into the issue. I will agree that with simple prompting and without reminders, models do not do this sort of thing well or consistently. On the other hand I have two skills that I use regularly `/atomic-commits` and `/split-branch` that do exactly what you're describing. It has been an exercise in taking the time to put to words the concepts you're describing, which has been very interesting because it's forced me to put to words many thoughts on the subject I had never formalized, and you're correct, there's a lot of theory of mind and considering of other people's perspectives that go into it.

* Does the series of commits tell a cohesive story of the feature, each commit building on the last one? It's surprisingly easy to have an ordering of commits where each one builds, but the ordering is nonsense. * Is each commit scoped to the right size? Not too big of course, describes a single idea or concept. That same rule can also lead to commits that are comically small or separates two changes that can obviously be argued are "two different things" but that make more sense when seen together as a single conceptual change. * When is it appropriate to split up disparate changes within a single file, interwoven together, and how to correctly orchestrate that with git commands.

I care _deeply_ about commit hygiene and presentation because it's a foundation for making code review a smooth process and in the Before Times™ I would spend a _substantial_ amount of time reworking my own commits to ensure they were up to my own standards. I can count on two hands the number of commits I've authored on my own in the last year, and the process is immediately replicated and standardized across our entire engineering organization.

It is perfect 100% of the time? No, every once in a while I'll hit some weird case and need to issue some corrective instructions, or more likely ask the model why things are like that to see if there's tuning we need to do.

Commits are effectively like automated code-formatting now. This isn't years in the future, this is already _old news_.

Re: Stop sending me huge PRs; a rant

#65

Post by throwaway2037 »

sackfield wrote: post #61

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 con…

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.

Re: Stop sending me huge PRs; a rant

#66

Post by Seattle3503 »

stackskipton wrote: post #5

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.

> If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.

Approvals are probably required by SOC 2 or other compliance requirements.

Re: Stop sending me huge PRs; a rant

#67

Post by throwaway2037 »

usewik wrote: post #11

> 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.

    > 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.

Re: Stop sending me huge PRs; a rant

#68

Post by throwaway2037 »

ok_computer wrote: post #7
ventana wrote: post #2

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 th…

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.

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.

Re: Stop sending me huge PRs; a rant

#69

Post by throwaway2037 »

dboreham wrote: post #43

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…

    > 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."

Re: Stop sending me huge PRs; a rant

#70

Post by packetlost »

garfij wrote: post #64
danpalmer wrote: post #8

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,…

I have a completely different experience than you, but I have also invested quite a bit of effort into the issue. I will agree that with simple prompting and without reminders, models do not do this sort of thing well or consistently. On the other hand I have two skills that I use regularly `/atomic-commits` and `/split-branch` that do exactly what you're describing. It has been an exercise in taking the time to put…

I've also had a ton of luck getting agents to make focused, (mostly) well crafted commits using my home-grown git-based task manager: https://github.com/chiefnoah/tsk

It's very easy to be like: /tsk . Break down the problem into focused tasks using tsk, include all context necessary to complete a task in the tasks body, then prioritize them. Then begin working on them in priority order until complete.

Works 9/10 for me, though I often split up the instructions a bit so I have time to review the resulting tasks/design. tsk itself encourages creating single commits per task because it tracks the commit a task is closed on and the agents are pretty good about doing that.

Post reply on HN