[jvans]

> but when you need to go back and re-open 4, 5 merged PRs in different tabs to get the full context again, the time to review increases tenfold with tiny PRs

You can't really say much about the first 4 PRs because if you don't know how that code is going to be used, it's kind of hard to giving meaningful feedback. By PR 5, it's too late to give feedback on PRs 1-4.

I also have worked with people that prefer that but I never understood it. Code without context isn't reviewable imo

[klodolph]

A skill that you see in some of the more experienced devs is the ability to front-load the most important stuff in a series of changes, or make changes that can at least be understood in isolation.

It’s not always reasonable to do that, but you can often deliver incremental changes where each incremental change either brings a whole feature visibly closer to completion, or provides some self-contained utility, or makes some refactoring changes with the promise of “this will make it possible to do X, later”.

[starttoaster]

That's a talented approach, yes. But something I've observed in a couple of former coworkers is they will put together 6 PRs to the main branch where the code won't even (purposefully) build yet. By PR 6, the code functions, but is in a terrible state because PRs 1-5 were essentially pointless to review, and many of the issues you have with the code base are now firmly outside of the 6th PR's diff. I've likened this phenomena to the H.H. Holmes murder hotel, where he hired contractors to do extremely small jobs individually, and none of them realized (or at least had some plausible deniability) that they were creating a hotel made perfectly for the owner to murder residents. Except in this situation, I perceive the author was hiding the fact that they have no clue how to write code, which I don't know who they thought they fooled.

[johannes1234321]

Right, in my experience large changesets often don't add a single thing, but do some refactoring, which enables the change, and then add some new infrastructure and the individual feature.

If you split it, you can verify the refactoring is a pure refactoring, not changing functionality (much), can understnad the infrastructure added and then understand the feature built on top.

Of course often things aren't developed in isolation, thus preparing the review for isolating those aspects is work again, but usually that in a side effect leads to better architecture, as you look at the parts in isolation, leading to half a refactoring, half an infrastructure and a feature hacked in.

[jvans]

Definitely. The good version of small PRs are small but useful changes that incrementally produce value on their own

The bad version is people adding random unused classes and methods that don't make sense until the final PR comes in and uses them