Phabricator for patches and code review

[Simon Peyton Jones]

PS I couldn't get past the login box at https://phabricator.haskell.org/D4

| -----Original Message-----
| From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of Austin
| Seipp
| Sent: 06 June 2014 05:06
| To: ghc-devs at haskell.org
| Subject: RFC: Phabricator for patches and code review
| 
| 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