Some of us like "interdiff" code review
gist.github.com
Some of us like "interdiff" code review
1–10 of 224 posts
Re: Some of us like "interdiff" code review
#2I've generally found that code review first, and rebase-centric systems like Gerrit tend to be much easier to review code in. One of the best parts of this is native support for stacking multiple patches, so people make smaller patches that are easier to review.
Code review in Github feels like a bad afterthought - the space-wasting interface that looks more like a forum thread, the inability to track over rebases, etc.
Re: Some of us like "interdiff" code review
#3Someone should do a deep dive into developer productivity after LLVM switched from Phabricator to GitHub. How many other major projects have done a switch like that?
Re: Some of us like "interdiff" code review
#4Most of the complaints here could be solved by having smaller pull requests and then squashing commits when it’s time to merge.
Re: Some of us like "interdiff" code review
#5Most of the complaints here could be solved by having smaller pull requests and then squashing commits when it’s time to merge.
Well, yes. Or by having stacked branches and rebasing your branches, etc.
The point is that GitHub/git's default experience makes this harder to do than a system that bakes it in.
Re: Some of us like "interdiff" code review
#6I create my GitHub PRs like a reviewer's going to look at the commits if it's too large overall. I'm also fairly sure they don't, because it's basically never worth doing so in their PRs ('fix the test', 'merge origin/master', 'address review comment', etc. commits).
I suppose I agree GitHub doesn't help with this / implicitly opposes it (I don't see the 'explicitly and' claim justified though?) but there's nothing about a PR that isn't a 'series' of 'patches'. Maybe GitHub's just responding to the way most people use it, and making that easier/better instead of being principled? Doesn't mean we can't be.
Re: Some of us like "interdiff" code review
#7Nice, this taught me about `git range-diff` which wasn't on my radar before.
Is the conclusion likely to be that the author thinks Gerrit is good, or is there some nuance I didn't pick up? I've used Gerrit before and in hindsight I much prefer it to other ways of doing code review.
Re: Some of us like "interdiff" code review
#8100% agree that this is ideal, the way Github does it is completely godawful and it's a tragedy that so many people have it normalized for them.
We did this with Phabricator, although it was a somewhat-manual process, helped along by having some command line macros for updating all the reviews at once. But better still would be an explicit UI for it.
Re: Some of us like "interdiff" code review
#9Most of the complaints here could be solved by having smaller pull requests and then squashing commits when it’s time to merge.
We use stacked commits + rebase only in our company. The commit history is linear and it's very easy to revert changes. I don't see any advantage of using merging instead of rebase
I am not sure why we need to squash commits. We encourage the opposite where you should commit small and often. So if we need to revert any commit, it's less painful to do so.
Re: Some of us like "interdiff" code review
#10Someone should do a deep dive into developer productivity after LLVM switched from Phabricator to GitHub. How many other major projects have done a switch like that?
Better it be a deep dive into developer sanity, a more important but comparatively under-valued metric.