Proposal: accept pull requests on GitHub

Niklas Hambüchen mail at nh2.me
Tue Sep 1 20:42:21 UTC 2015


Hi,

I would recommend against moving code reviews to Github.
I like it and use it all the time for my own projects, but for a large
project like GHC, its code reviews are too basic (comments get lost in
multi-round reviews), and its customisation an process enforcement is
too weak; but that has all been mentioned already on the
https://ghc.haskell.org/trac/ghc/wiki/WhyNotGitHub page you linked.

I do however recommend accepting pull requests via Github.

This is already the case for simple changes: In the past I asked Austin
"can you pull this from my branch on Github called XXX", and it went in
without problems and without me having to use arc locally.

But this process could be more automated:

For Ganeti (cluster manager made by Google, written largely in Haskell)
I built a tool (https://github.com/google/pull-request-mailer) that
listens for pull requests and sends them to the mailing list (Ganeti's
preferred way of accepting patches and doing reviews). We built it
because some people (me included) liked the Github workflow (push
branch, click button) more than `git format-patch`+`git send-email`. You
can see an example at https://github.com/ganeti/ganeti/pull/22.
The tool then replies on Github that discussion of the change please be
held on the mailing list. That has worked so far.
It can also handle force-pushes when a PR gets updated based on
feedback. Writing it and setting it up only took a few days.

I think it wouldn't be too difficult to do the same for GHC: A small
tool that imports Github PRs into Phabricator.

I don't like the arc user experience. It's modeled in the same way as
ReviewBoard, and just pushing a branch is easier in my opinion.

However, Phabricator is quite good as a review tool. Its inability to
review multiple commits is nasty, but I guess that'll be fixed at some
point. If not, such an import tool I suggest could to the squashing for you.

Unfortunately there is currently no open source review tool that can
handle reviewing entire branches AND multiple revisions of such
branches. It's possible to build them though, some companies have
internal review tools that do it and they work extremely well.

I believe that a simple automated import setup could address many of the
points in https://ghc.haskell.org/trac/ghc/wiki/WhyNotPhabricator.

Niklas

On 01/09/15 20:34, Thomas Miedema wrote:
> Hello all,
> 
> my arguments against Phabricator are here:
> https://ghc.haskell.org/trac/ghc/wiki/WhyNotPhabricator.
> 
> Some quotes from #ghc to pique your curiosity (there are some 50 more):
>  * "is arc broken today?"
>  * "arc is a frickin' mystery."
>  * "i have a theory that i've managed to create a revision that phab
> can't handle." 
>  * "Diffs just seem to be too expensive to create ... I can't blame
> contributors for not wanting to do this for every atomic change"
>  * "but seriously, we can't require this for contributing to GHC... the
> entry barrier is already high enough" 
> 
> GitHub has side-by-side diffs
> <https://github.com/blog/1884-introducing-split-diffs> nowadays, and
> Travis-CI can run `./validate --fast` comfortably
> <https://travis-ci.org/ghc/ghc/builds>.
> 
> *Proposal: accept pull requests from contributors on
> https://github.com/ghc/ghc.*
> 
> Details:
>  * use Travis-CI to validate pull requests.
>  * keep using the Trac issue tracker (contributors are encouraged to put
> a link to their pull-request in the 'Differential Revisions' field).
>  * keep using the Trac wiki.
>  * in discussions on GitHub, use https://ghc.haskell.org/ticket/1234 to
> refer to Trac ticket 1234. The shortcut #1234 only works on Trac itself.
>  * keep pushing to git.haskell.org <http://git.haskell.org>, where the
> existing Git receive hooks can do their job keeping tabs, trailing
> whitespace and dangling submodule references out, notify Trac and send
> emails. Committers close pull-requests manually, just like they do Trac
> tickets.
>  * keep running Phabricator for as long as necessary.
>  * mention that pull requests are accepted on
> https://ghc.haskell.org/trac/ghc/wiki/WorkingConventions/FixingBugs.
> 
> My expectation is that the majority of patches will start coming in via
> pull requests, the number of contributions will go up, commits will be
> smaller, and there will be more of them per pull request (contributors
> will be able to put style changes and refactorings into separate
> commits, without jumping through a bunch of hoops).
> 
> Reviewers will get many more emails. Other arguments against GitHub are
> here: https://ghc.haskell.org/trac/ghc/wiki/WhyNotGitHub.
> 
> I probably missed a few things, so fire away.
> 
> Thanks,
> Thomas
> 
> 
> 
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
> 


More information about the ghc-devs mailing list