3 ms·
> If reviewing huge PR's is a burden, I will make better tooling for it. Systems like Gerrit and Graphite already have that, because they show you an increment
by aseipp 3y ago
> If reviewing huge PR's is a burden, I will make better tooling for it.
Systems like Gerrit and Graphite already have that, because they show you an incremental diff between versions of the change. If you have a change with 500 lines, you only typically read that whole thing once. Incremental modifications result in incremental diffs, so if it takes 50 lines of code to address review comments, the reviewer only needs to read that 50 line change. This is loosely the same as git range-diff. I talked about this here: https://news.ycombinator.com/item?id=38371748 https://news.ycombinator.com/item?id=38371748
> What I don't want is a big feature to be split into 10 PR's, where the first 9 provide no user benefit and are just refactors or adding dead code ready for the last PR.
These tools do not use "PRs." They do not use branches. They use changes. You are not proposing that a branch is merged. You're proposing a single, atomic change (one that can pass integration and the testsuite), to the codebase. You review individual changes. You may review a sequence of related changes, but you do not review "branches", and the tool largely ignores branches. This is a subtle but very important distinction.
Tools like Gerrit or Graphite will show you the "relation chain" which is basically just the child/parent relationship between these changes. So, if you have 3 commits, those become 3 changes to review in the UI, and at any point, while looking at any of them, you can see the other 2 changes for context, quickly. You can do something similar on GitHub. Go to a multi-commit PR on github. Click any of the commits. You can now hit "Next" (shortcut 'n') or "Prev" (shortcut 'p') in order to flip between commits, like the pages of a book. This is a review style that some projects adopt.
In reality, stacking commits "well" requires a bit of practice. It's a "garbage in, garbage out" process, so if you're just throwing 50 changes up there a day it isn't going to work well. There is a bit of an art and also a science to make concise, easily readable and easily reviewable changes.
In practice I'd say the "ideal stack" is anywhere from 2-7 commits; line counts I don't think are important. (Sometimes a single atomic change really does need to be 500 lines.) 7 isn't a magical number. It's just that having too large of a stack is often easy to lose track of. But that's roughly the same problem as a branch on Github that has 80 commits or whatever.