Proposal: accept pull requests on GitHub

Greg Weber greg at gregweber.info
Wed Sep 2 15:43:02 UTC 2015


I like Niklas's suggestion of a middle-ground approach. There are benefits
to using phabricator (and arc), but there should be a lowered-bar approach
where people can start contributing through github (even though they may be
forced to do the code review on phabricator).

On Tue, Sep 1, 2015 at 1:42 PM, Niklas Hambüchen <mail at nh2.me> wrote:

> 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
> >
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20150902/c7621879/attachment.html>


More information about the ghc-devs mailing list