RFC: Phabricator for patches and code review

[Austin Seipp]

Hello all,

Recently, while doing server maintenance, several of the
administrators for Haskell.org set up an instance of Phabricator[1],
located at https://phabricator.haskell.org

For those who aren't aware, Phabricator (or "Phab") is a suite of
tools for software development. Think of it like a polished,
semi-private GitHub with a lot of applications and tools for all kinds
of needs. We've been using it to do issue tracking for Haskell.org
maintenance and like it a lot so far.

One very nice aspect of Phabricator though is it has a very nice code
review tool, called 'Differential', that is very useful. For people
who have used a tool like Review Board, it's similar. Furthermore, it
has a very convenient userland tool called 'Arcanist' which makes it
easy for newcomers to post a review and get it merged when it's ready
all from the command line.

I'd like to see if people are interested in using Phab _strictly_ for
code review of GHC patches. It is a dedicated tool specifically for
this, and I think it works much better than Trac or inline GitHub
comments.

Also, Phab can also support post-commit reviews. So if I touch
something in the runtime system and just push, perhaps Simon or Edward
would like to look, and they can be alerted right when I do this, and
then yell if I did something stupid.

Before I go much further, I'd like to ask: is there *any* interest in
this? Or are people satisifed with Trac? The primary motivations are
roughly, in no particular order:

 1) Code review is good for everyone, a good way for people to learn
the code and ask questions, and useful to give feedback to newcomers.
And even experienced GHC hackers can learn things from reading code,
as we all do regularly, or find things that need cleanup.

 2) Phabricator in particular makes it very easy to submit patches for
review. To submit a patch, I just run the command 'arc diff' and it
Does The Right Thing. It also makes it easy to ensure people are
*alerted* when a patch might be relevant to them.

 3) They can be uploaded and created from the command line, and merged
easily afterwords the same way. This is particularly useful for
newcomers, and for me. :)

 4) Differential is dedicated to code review, and much better at it
than just reading patches on Trac IMO.

 5) It supports both post-commit code review, as well as pre-commit
review. Post commit would be especially useful for us too, I think.

Point #2 and #3 are mostly relevant for me, because I mostly handle
incoming patches. But I think in general it would be nice, and make it
a lot easier for newcomers to submit patches, and us to look over
them.

Here's an example of a Differential code review:

https://phabricator.haskell.org/D4

This is a demo using my 'wip/ermsb' patch. You'll need to create an
account to login, but it shouldn't be much trouble, you can login
several ways. I'll fix the login requirement soon. Feel free to read
the code, comment on it, and play around. It's more of a
demonstration, but real code review would be welcome too. :)

If people are interested in doing this, I can add notes to the wiki
pages for newcomers, and I'll send another email about Phab so people
can understand it a little better. But I want to ask first.

There is an argument that our team is so small, code review has
unnecessary burdens. But I think Phab could help a lot with tracking
outside patches and getting good reviews for incoming patches, and
it'll make it easier for newcomers. And experienced pros can probably
learn a thing as well.

Again, to be clear, I don't propose we migrate anything to Phabricator
from, say, Trac. There's no real pressure to do so and it would be
tons of work. I only propose we use it for code review, which is
perfectly fine, and how other projects like LLVM do code review (they
use Bugzilla).

I also don't think the usage of Phabricator should be mandatory
(unless we decide that later because we like it), but I would like to
see people use it if possible.

[1] http://phabricator.org

-- 
Regards,

Austin Seipp, Haskell Consultant
Well-Typed LLP, http://www.well-typed.com/