Hacker News Reader

Comment by rokkamokka | Hacker News Reader

I'm on the fence about this. Sometimes a new feature needs maybe 2k lines of code split over 10-20 or so files. Sure, you could split it up, but you can't necessarily run the parts in isolation and if they get split over multiple reviewers it might even be that no one reviewer gets the whole context.

So, I'm kind of okay with large PRs as long as they're one logical unit. Or, maybe rather, if it would be less painful to review as one PR rather than several.

View on HN

I think the particular problem is if AI is just producing large volumes of code which are unnecessary, because the LLM is simply not able to create a more concise solution. If this is the case it suggests these LLM generated solutions are likely bringing about a lot of tech debt faster than anyone is ever likely to be able to resolve it. Although maybe people are banking on LLMs themselves one day being sophisticated enough to do it, although that would also be the perfect time to price gouge them.

Agree. We've seen cowboy developers who move fast by producing unreadable code and cutting every corner. And sometimes that's ok. Say you want a proof of concept to validate demand and iterate on feedback. But we want maintainable and reliable production code we can reason about and grasp quickly. Tech debt has a price to pay and looks like LLM abusers are on a path to waking up with a heavy hangover :)

We hired some LLM cowboy developer externals that were pushing out a plethora of PRs daily and a large portion of our team's time at one point was dedicated entirely to just doing PR reviews. Eventually we let them go, and the last few months for us has been dedicated to cleaning up vast quantities of unmaintainable LLM code that's entered our codebase.

I think it's still early days, and it's probably the case that a lot of software development teams have yet to realize that a team basically just doing PR reviews is a strong indication that a codebase is very quickly trending away from maintainability. Our team is still heavily using LLMs and coding agents, but our PR backlog recently has been very manageable.

I suspect we'll start seeing a lot of teams realize they're inundated with tech debt as soon as it becomes difficult for even LLMs to maintain their codebases. The "go fast and spit out as much code as humanly possible" trend that I think has infected software development will eventually come back to bite quite a few companies.

Yep, it's the early days. Eventually we'll work out something like Design Patterns for Hybrid Development, where humans are responsible for software architecture, breaking requirements into maintainable SOLID components, and defining pass/fail criteria. Armed with that, LLMs will do the actual boilerplate implementation and serve as our Rubber Ducky Council for Brainstorming :)

I'm okay with long PRs, but please split them up into shorter commits. One big logical unit can nearly always be split into a few smaller units - tests, helpers, preliminary refactoring, handling the happy vs error path, etc. can be separated out from the rest of the code.

There really is no benefit of splitting a functionality from it's test. Then you just have a commit in the history which is not covered by tests.

Splitting "handling the happy vs error path" sounds even worse. Now I first have to review something that's obviously wrong (lack of error handling). That would commit code that is just wrong.

What is next, separating the idea from making it typecheck?

One should split commits into the minimum size that makes sense, not smaller.

"Makes sense" should be "passes tests, is useful for git bisect" etc, not "has less lines than arbitrary number I personally like to review" - use a proper review tool to help with long reviews.

Depends entirely on your workflow - we squash PRs into a single commit, so breaking a PR into pieces is functionally identical to not doing so for the purposes of the commit history. It does, however, make it easier to follow from the reviewer's perspective.

Don't give me 2000 lines unless you've made an honest good-faith attempt to break it up, and if it really can't be broken up into smaller units that make sense, at least break it up into units that let me see the progression of your thought as you solve the problem.

> Sometimes a new feature needs maybe 2k lines of code split over 10-20 or so files

I still disagree. Why was the feature not split up into more low-level details? I don't trust that kind of project management to really know what it's doing either.

I am not promoting micromanagement, but any large code review means the dev is having to make a lot of independent decisions. These may be the right decisions, but there's still a lack of communication happening.

Hands off management can be good for creativity and team trust, but ultimately still bad for the outcome. I'm speaking from my own experience here. I would never go back to working somewhere not very collaborative.

This is very much my take. As long as the general rule is a lack of long PRs, I think we get into a good place. Blueskying, scaffolding, all sorts of things reasonably end up in long PRs.

But, it becomes incumbent on the author to write a guide for reviewing the request, to call the reviewer's attention to areas of interest, perhaps even to outline decisions made.

> on the author to write a guide for reviewing the request

I'm not saying that doesn't work, but writing a guide means the author is now also doing all the planning too.

A successful "guide" then becomes more about convincing the reviewer. The outcome is either a lot of friction, or the reviewer is just going through the motions and trust is eroding.

That's fine but such a PR doesn't need to be (and actually can't) be reviewed. Or at least it can only be reviewed broadly: does it change files that shouldn't change, does it have appropriate tests, etc.

Maybe AI should not author big and complex features that cant be split up into parts and thus easily reviewed.

Reply on news.ycombinator.com