Can only view diffs on a single page (can be very slow).
I've seen GitHub take five seconds to render a 100K line diff. In my experience all of the other tools I've used, including some of the ones listed, can take longer to render individual file sections of such a diff. It's fast enough.
Comments are sent as they are written; you cannot "draft" comments.
I'm a bit puzzled as to why you would need to draft comments inside the PR interface, especially in light of the fact that they can be edited.
Cannot compare differences between patch sets.
You most certainly can. Just use branch/revision compare.
To create a patch one must fork the repository publicly (weird and unnecessary).
Also immaterial.
Accepting a patch creates a "merge commit" (ugly repo history).
This comes closest to being a legitimate complaint. Because GitHub emphasizes recognition of contributors, it doesn't let you rewrite the commits in the PR as you merge them with the merge button, but there are multiple ways to merge on the command line that avoid the merge commit and play nice with the PR.
In general, pull request culture is not about code review.
> I'm a bit puzzled as to why you would need to draft comments inside the PR interface, especially in light of the fact that they can be edited.
This scenario happens often: I read through a change, making comments as I go. Then I reach some part of the change and realise "Oh, that explains why they did that in that other file!" So I go back and delete or alter my comments.
In Gerrit or Rietveld, the reviewee never sees those earlier comments.
On GitHub, the reviewee has already received the comment notifications and started responding before I have a chance to make the changes. The reviewee wastes time responding to questions to which I already know the answer. It's clunky, inefficient, and unnecessary.
It seems like you haven't used a tool like Gerrit or Rietveld. You should check them out.
A lot of people commenting here probably haven't had the experience of working with mature pre-submit code review tools. It's funny how much of an opinion people seem to have about things they don't know about.
I'm shocked at how bad Githubs PR review UI is given how much funding they have had for so many years. Even abandoned side projects like Rietveld have vastly better review UIs and workflows for larger patch sets.
I probably sounded overly dismissive of github. Which probably isn't fair. But I'll leave my post un-edited for posterity.
I use github every day for work. There are lots of things it gets right. But if you want to work on a project where you more or less have a central repository, take contributions from external and internal contributors, and have a strict policy of pre-submit reviews for individual commits to master that vary in size and complexity, then you want a tool that is 'code review centric'. That is, something that supports comment drafts, back and forth exchanges across many files, and notions of iterations as the reviewee responds to feedback. Github is lacking in this regard, for all the reasons the Go team pointed out. I mean... they just recently shipped side by side diffs!
The main thrust of my comment was to point out that lots of people in this thread are criticizing the Go team's choice to use Gerrit instead of Github, and the points they make seem to stem from an ignorance of both tools like Gerrit, and of workflows that work well that aren't pull requests. It's a bit of "what you see is all there is" where all they've seen is Github. And statements like "github is good enough" seem overly dismissive of the decisions of a lot of very smart people on the Go team.
For folks that are familiar with Gerrit and still poopoo their decision to use it. Well... we can agree to disagree :).
I'm rooting for Github. I want to see it get better so I can stop running a separate code review tool for my own projects! But it's got a ways to go still.
@piotrkaminski Comment nesting seemed to run out. So replying to myself.
Our team is currently using Rietveld. But I've used Gerrit in the past, as well as internal tools of the same flavor back when I was at Google.
I don't particularly love Rietveld. But it's simple to maintain and does the job. That being said, I'm genuinely looking forward to one day being able to just use Github for this.
Heh, that's almost exactly like me: used internal tools at Google, brought up a Rietveld instance after I left. Except that I got frustrated with Rietveld and built https://reviewable.io -- you might want to check it out. :)
FullStory looks awesome, BTW, I just wish I could afford it.
You might also want to check out https://reviewable.io. It's got most of the useful features of Rietveld (and, to a lesser degree, Gerrit), but with a UI that isn't stuck in the '90s and one-click integration with your GitHub project. Draft comments, threaded discussions that don't disappear, explicit tracking of resolution, quick diffs between (multi-commit) revisions, support for rebasing/merging without losing review history, etc. It trades off the deep customization possible with Gerrit, but is still much better than GitHub's PRs (IMHO -- I'm the author).
Thanks for replying directly. I have; I've used Gerrit and Phabricator (which also does inline comment draft keeping) a lot, and I've clicked around Rietveld. I just never have seen much use for the comment draft feature; keeping comments in draft is not what I naturally do. I can see where others might like that, so it could be a nice to have (though you can also use your browser to keep draft comments).
I think GitHub and to a lesser extent Phabricator have enormous UI/UX upsides over Gerrit and Rietveld, which perpetually look like internal tools that never underwent proper UX review. What's more important is that GitHub has done so much for open source software and has achieved such critical mass that a decision to not use it now starts to also shut you off from a population of developers for whom the activation energy is too high.
You're talking about GitHub commit comments, right? Can you not simply click to comment, compose your comments one by one, and then not send them immediately? You can send them all together after you finish reviewing the entire commit.
Although it would be nice to have a 'Send All Comments' button at the bottom of the commit page so you didn't have to click through them one by one.
I've seen GitHub take five seconds to render a 100K line diff. In my experience all of the other tools I've used, including some of the ones listed, can take longer to render individual file sections of such a diff. It's fast enough.
Comments are sent as they are written; you cannot "draft" comments.
I'm a bit puzzled as to why you would need to draft comments inside the PR interface, especially in light of the fact that they can be edited.
Cannot compare differences between patch sets.
You most certainly can. Just use branch/revision compare.
To create a patch one must fork the repository publicly (weird and unnecessary).
Also immaterial.
Accepting a patch creates a "merge commit" (ugly repo history).
This comes closest to being a legitimate complaint. Because GitHub emphasizes recognition of contributors, it doesn't let you rewrite the commits in the PR as you merge them with the merge button, but there are multiple ways to merge on the command line that avoid the merge commit and play nice with the PR.
In general, pull request culture is not about code review.
WTF?
Sounds like a strong case of NIH.