Hacker News Reader

Comment by ActionHank | Hacker News Reader

I dunno man, I usually close the PR when someone does that and tell them to make more atomic changes.

Would you trust an colleague who is over confident, lies all the time, and then pushes a huge PR? I wouldn't.

Closing someone else’s PR is an actively hostile move. Opening a 20k LOC isn’t great either, but going ahead and closing it is rude as hell.

Dumping a huge PR across a shared codebase wherein everyone else also has to deal with the risk of you monumental changes is pretty rude as well, I would even go so far as to say that it is likely selfishly risky.

Opening a 20k LOC PR is an actively hostile move worthy of an appropriate response.

Closed > will not review > make more atomic changes.

A 20k LOC PR isn’t reviewable in any normal workflow/process.

The only moves are refusing to review it, taking it up the chain of authority, or rubber stamping it with a note to the effect that it’s effectively unreviewable so rubber stamping must be the desired outcome.

Sure it is if the great majority is tests.

I don't understand this attitude. Tests are important parts of the codebase. Poorly written tests are a frequent source of headaches in my experience, either by encoding incorrect assumptions, lying about what they're testing, giving a false sense of security, adding friction to architectural changes/refactors, etc. I would never want to review even 2k lines of test changes in one go.

Preach. Also, don't forget making local testing/CI take longer to run, which costs you both compute and developer context switching.

I've heard people rave about LLMs for writing tests, so I tried having Claude Code generate some tests for a bug I fixed against some autosave functionality - (every 200ms, the auto-saver should initiate a save if the last change was in the previous 200ms). Claude wrote five tests that each waited 200ms (!) adding a needless entire second to the run-time of my test suite.

I went in to fix it by mocking out time, and in the process realized that the feature was doing a time stamp comparisons when a simpler/non-error prone approach was to increment a logical clock for each change instead.

The tests I've seen Claude write vary from junior-level to flat-out-bad. Tests are often the first consumer of a new interface, and delegating them to an LLM means you don't experience the ergonomics of the thing you just wrote.

i think the general take away for all of this is the model can write the code but you still have to design it. I don't disagree with anything you've said, and I'd say my advice is engage more, iterate more, and work in small steps to get the right patterns and rules laid out. It wont work well on day one if you don't set up the right guidelines and guardrails. That's why it's still software engineering, despite being a different interaction medium.

And if the 10k lines of tests are all garbage, now what? Because tests are the 1 place you absolutely should not delegate to AI outside of setting up the boilerplate/descriptions.

If somebody did this, it means they ignored their team's conventions and offloaded work onto colleagues for their own convenience. Being considered rude by the offender is not a concern of mine when dealing with a report who pulls this kind of antisocial crap.

Dumping a 20k LOC PR on somebody to review especially if all/a lot of it was generated with AI is disrespectful. The appropriate response is to kick that back and tell them to make it more digestible.

I'm the owner of some of my work projects/repos. I will absolutely without a 2nd thought close a 20k LoC PR, especially an AI generated one, because the code that ends up in master is ultimately my responsibility. Unless it's something like a repo-wide linter change or whatever, there's literally never a reason to have such a massive PR. Break it down, I don't care if it ends up being 200 disparate PRs, that's actually possible to properly review compared to a single 20k line PR.

Reply on news.ycombinator.com