logoalt Hacker News

esailijatoday at 9:50 AM2 repliesview on HN

It isn't about small or big, it's about cohesion of the changes.

I prefer a big feature to be one big PR rather than a lot of small ones.

We had a dev do a big feature with a ton of small PRs, each one was individually impossible to review because each concern was out of scope for the small PR and "would be fixed in later PRs". Once it all came together as as whole, the big picture was a total horror show and I had to rewrite basically the whole thing.

In order to review those small PRs properly, each time I would have to read and understand all the current code so far from the beginning. Without that, each small PR individually looks OK because you won't remember the other PRs from weeks back that already duplicated what the current small PR does for example.


Replies

sgarlandtoday at 2:01 PM

Fully agree. If you want to follow the thought process through a large PR, review each commit (assuming, of course, the author made reasonable commits) on its own.

enraged_cameltoday at 11:27 AM

>> I prefer a big feature to be one big PR rather than a lot of small ones.

Yes, same, and I genuinely do not understand the insistence that PRs should not be above a certain size. I think most people are under the (misguided and wrong) impression that a PR review should take less than the time it took to write the code, and therefore allocate no more than 15-30 minutes per review. So when they come across a large PR they find themselves at a loss.