Having been using Github internally for code reviews of private patches on LLVM, and Phabricator for upstream ones, I’ve found the latter to be far easier to use. Prior to working with LLVM, I had basically no experience with either, so I’d say I’m coming from a fairly neutral starting point. Here are some of my observations (I want to highlight 9 as a particular issue, due to other recent discussions I’ve seen on the mailing list):
-
I don’t know of a natural way to chain related patches together on Github (aside from explicitly mentioning them), but as separate reviews. This is useful for bigger features/refactorings where each individual step provides some benefit, but seeing the bigger picture is easy. Phabricator has the child/parent objects option.
-
Phabricator allows placing comments anywhere in the diff context, which is useful when the commit affects lines that haven’t actually been changed, and those lines need addressing/referencing in some way. Github doesn’t allow comments outside the immediate context around changed files
-
As a +1 to Github on the other hand, commits always come with full context, so you don’t have to remind people to include it.
-
Phabricator’s ability to see what has changed since the previous time you commented seems to be much more reliable than what Github provides.
-
With a Github PR, if you want to include a minor change to the patch prior to committing, you have to commit it to the PR, if you wish to use the UI to do the merging. Of course, you could just not push the patch via Github.
-
I myself have on numerous occasions messed up my commit message when committing via the Github UI, because it’s not obvious unless you are a seasoned user that the title of the commit appears as the first line of the commit message. This is important when doing a squash and merge.
-
The PR approach does at least allow committing via the UI, which is perhaps a little less fiddly in some cases.
-
I rarely bother creating a branch for Phabricator reviews, because I don’t need it (I’m often not working on multiple things at once), so the extra hassle of creating a branch/checking it out etc that Github requires is annoying. On the other hand, Github PRs are generally quite easy to create once you have pushed a branch.
-
On the note of branches for PRs, don’t they require users to push their local branches to the remote repo to create? That means we’ll end up thousands of branches in git. Not sure that this will do performance any good, and I seem to remember there was general agreement that we didn’t want people to push their branches generally. Yes, in theory branches should be deleted after they’re merged, but I’ve seen that locally not happen regularly, and that’s even assuming that all PRs get merged in (they won’t).
-
Expanding context on Github is a pain: there is no option to just expand the whole context in a block, which means that if you need to see something much earlier in a large file, you have to click a LOT. Also, the browser view often doesn’t then go where you expect it to from my experience. Phabricator has a “expand all N lines” option.
-
Small one this one, but missing new lines at end of file are much more obvious on Phabricator than Github.
-
Not sure if this is a real issue, but Github reviewers are limited in number (I think it’s 15?). To my knowledge, there is no such limit with Phabricator (but then how often do you end up with 15 people marked as reviewers?).
I’m sure I could come up with other points for/against Github PRs. On balance I definitely prefer Phabricator.
James