RFC: Phabricator for patches and code review

[Carter Schonwald]

while i'm a novice at using ANY code review tools, having some persistent
tooling for patch reviews would be really great! theres a lot of good
feedback that otherwise only exists in an email somewhere!


On Fri, Jun 6, 2014 at 12:05 AM, Austin Seipp <austin at well-typed.com> wrote:

> 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/
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://www.haskell.org/mailman/listinfo/ghc-devs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140606/1c9dc472/attachment.html>