Create a ghc-simple-patch-propose list? Re: Notes from Ben's "contribute to ghc" discussion

Matthew Pickering matthewtpickering at gmail.com
Thu Sep 29 21:33:57 UTC 2016 

Thanks for the useful data point Mathieu. I think it should also be
noted that GHC contributions spiked after switching to phabricator so
it could just be the effect of moving to *some* code review tool.
Could you perhaps summarise the salient points in the LLVM thread? It
is very long with lots of discussion points.

FWIW, I think that we should accept pull requests but only if they are
mirrored and reviewed on phabricator.

This is important for three reasons:

1. Reviewers only need to look in one place and can clearly see when
they are expected to review. Herald rules will also not trigger if a
pull request is solely dealt with on github
which could mean the right eyes don't land on the ticket.
2. Pull requests (differentials) are able to be referred to by their
unique differential number rather than the pull request identifier
which clashes with trac ticket numbers.
3. Differentials integrate (and are able to be integrated) with the
existing infrastructure. I hope that in the future we improve this
integration by moving the issue tracker to maniphest.

Facebook have a bot which does the mirroring for some of their
repositories but it isn't open-sourced.

I believe we should be trying to consolidate rather than spread
everything even thinner.

Matt



On Thu, Sep 29, 2016 at 9:55 PM, Boespflug, Mathieu <m at tweag.io> wrote:
> Hi Richard!
>
> thanks for creating the pull request with a full proposal. You beat me to it
> - tried writing up much the same before stopping for dinner, essentially
> capturing just one of the points in Moritz's earlier (large) proposal.
> Moritz, I would encourage you like Richard did earlier to split the
> remaining points in your proposal into separate PR's too.
>
> So Richard, I created a PR to your PR to add in a little bit more detail:
> keeping the two mirrors in sync, role of the release manager to ensure that
> adequate reviewers get identified so that patches don't go unnoticed by an
> interested party who'd specifically want to review the patch on Phabricator,
> etc.
>
> https://github.com/goldfirere/ghc-proposals/pull/1
>
> I also moved one of the two choices you mention to the "alternatives"
> section of the document, thinking that's the pupose of the section.
>
> On another note, I solicited an experience report from Gabriel Scherer
> earlier this week. The Ocaml community likewise pondered using GitHub back
> in 2014, and in fact lauched an experiment to that effect.  So I was curious
> to hear how it went after 2 years of data.
>
> You can see the announcement here:
>
> http://thread.gmane.org/gmane.comp.lang.ocaml.platform/30
>
> Gabriel tells me that the initial situation for Ocaml was different from
> that of GHC: they had no formal code review tool in use, but would swap
> around patches on the Mantis issue tracker. Be it as it may, it's
> interesting to note just how much the number of contributions to Ocaml has
> increased in recent years, after this experiment started:
>
> https://github.com/ocaml/ocaml/graphs/contributors
>
> This experiment is today considered a big success. A key ingredient I gather
> is that Gabriel volunteered to triage the GitHub PR's and play the
> go-between to make sure all Mantis users were aware of any proposed changes
> relevant to them.
>
> The key insight I would put forth here is that how-to-accept-patches and
> where-to-review-them should be an agreement between the contributor and the
> assigned reviewer(s), in particular for trivial changes. For any given
> patch, provided the reviewer(s) is/are game, and provided all protential
> reviewers are made aware of its existence, it shouldn't matter much whether
> the patch came from Trac, Phabricator or GitHub.
>
> PS: Gabriel was very kind to also point out that the LLVM community has been
> big users of Phabricator and pondering introducing GitHub into the mix too.
> The discussion there should be relevant to this thread:
>
> http://lists.llvm.org/pipermail/llvm-dev/2016-May/100310.html
>
> Best,
>
> --
> Mathieu Boespflug
> Founder at http://tweag.io.
>
> --
> Mathieu Boespflug
> Founder at http://tweag.io.
>
> On 29 September 2016 at 20:55, Richard Eisenberg <rae at cs.brynmawr.edu>
> wrote:
>>
>> I have tried to gather the ideas from this thread into a formal proposal:
>> https://github.com/ghc-proposals/ghc-proposals/pull/11
>>
>> Please feel free to make suggestions to improve this, especially if I've
>> captured anyone's contributions incorrectly.
>>
>> -=-=-=-=-=-=-=-=-=-=-
>> Richard A. Eisenberg
>> Asst. Prof. of Computer Science
>> Bryn Mawr College
>> Bryn Mawr, PA, USA
>> cs.brynmawr.edu/~rae
>>
>> > On Sep 29, 2016, at 10:20 AM, Christopher Allen <cma at bitemyapp.com>
>> > wrote:
>> >
>> >> Instead perhaps GitHub's new review system may be the way forward for
>> >> GHC. It allows you to easily use git in the way it's meant to be used.
>> >
>> > Many problems are caused by letting your inner tinkerer/genius tailor
>> > dictate how things should be dealt with. Better to cut the gordian
>> > knot. I think Michael's right.
>> >
>> > On Thu, Sep 29, 2016 at 5:05 AM, Michael Sloan <mgsloan at gmail.com>
>> > wrote:
>> >>
>> >>
>> >> On Wednesday, September 28, 2016, Eric Seidel <eric at seidel.io> wrote:
>> >>>
>> >>> On Wed, Sep 28, 2016, at 18:37, Ben Gamari wrote:
>> >>>> Moritz Angermann <moritz at lichtzwerge.de> writes:
>> >>>>
>> >>>>> All that arc essentially does is, compute the diff from an offset
>> >>>>> (e.g. master) to the current HEAD and upload that to a new or
>> >>>>> existing
>> >>>>> (--update) differential. It also adds some meta information about
>> >>>>> the
>> >>>>> range, such that arc patch supposedly knows into which commit to
>> >>>>> apply
>> >>>>> the patch to.
>> >>>>>
>> >>>> Sure, but this leads to generally unreviewable patches IMHO. In order
>> >>>> to
>> >>>> stay sane I generally split up my work into a set of standalone
>> >>>> patches
>> >>>> with git rebase and then create a Diff of each of these commits.
>> >>>> Phabricator supports this by having a notion of dependencies between
>> >>>> Diffs, but arcanist has no sensible scheme for taking a branch and
>> >>>> conveniently producing a series of Diffs.
>> >>>
>> >>> I completely understand how this would be frustrating for core
>> >>> contributors (more specifically for people submitting large patches),
>> >>> but for new or casual contributors it's actually quite freeing. I
>> >>> don't
>> >>> have to worry about how messy my local history gets, because arc will
>> >>> throw it all away regardless! It absolves me of an extra
>> >>> responsibility,
>> >>> and lowers the barrier to contributing.
>> >>
>> >>
>> >> I dislike this workflow because I am already used to doing a lot of git
>> >> rebasing / amending / auto squashing.  So using arc means taking away
>> >> my
>> >> ability to write multi commit stories of how the change was crafted.
>> >> For
>> >> large changes there are often multiple logical inter related steps.
>> >> Squashing them into one big commit makes it much harder to review.  I
>> >> can
>> >> easily do that myself by marking everything as squash in a rebase. It
>> >> feels
>> >> like arcanist is just taking away power, not giving it (note i have not
>> >> used
>> >> it much - voice of a newbie here)
>> >>
>> >> I am beginning to change my feelings on this, away from thinking of
>> >> GitHub
>> >> as an auxilliary source of didferentials.  Instead perhaps GitHub's new
>> >> review system may be the way forward for GHC. It allows you to easily
>> >> use
>> >> git in the way it's meant to be used.
>> >>
>> >> -Michael
>> >>
>> >>>
>> >>>
>> >>> It would be nice to support both workflows though :)
>> >>> _______________________________________________
>> >>> 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
>> >>
>> >
>> >
>> >
>> > --
>> > Chris Allen
>> > Currently working on http://haskellbook.com
>> > _______________________________________________
>> > 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
>
>
>
> _______________________________________________
> 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