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

David Turner dct25-561bs at mythic-beasts.com
Thu Sep 29 06:12:48 UTC 2016 

Hi,

You can alter the content of a GitHub PR after its initial creation. The
semantics of a PR is "please merge my branch named B into your repo" where
the branch B is a mutable pointer to a commit.

A workflow I've used a few times is to craft a nice sequence of commits for
review and, once accepted, updated the PR to a squashed version of the
same. Not that I like squashing, but that's what upstream wanted.

In fact, the mutability of a PR is something you have to be a bit careful
about on GitHub as you may not be accepting what you reviewed. It doesn't
seem very easy to see how the branch has changed over time: there is no
audit facility. I'd have thought the tooling discussed earlier should be
able to take care of this problem.

Cheers,

On 28 Sep 2016 09:44, "Simon Marlow" <marlowsd at gmail.com> wrote:

> Well, let's be careful here.  I like the idea, but it's not a complete
> solution for people who don't want to use arc, because you can't revise a
> patch after submission in response to reviews, you would have to open a new
> PR.
>
> Perhaps you could build something that would allow revisions to PRs too...
> that would be cool.
>
> Cheers
> Simon
>
> On 28 September 2016 at 03:22, Michael Sloan <mgsloan at gmail.com> wrote:
>
>> Exactly!  So we will be using Phabricator for the review process, but
>> with the github PRs you can use plain git.  This means that new
>> contributors will only need to learn about phabricator, and arc will
>> be non-mandatory though probably recommended.
>>
>> Glad you like the idea :)
>>
>> -Michael
>>
>> On Tue, Sep 27, 2016 at 6:47 PM, Richard Eisenberg <rae at cs.brynmawr.edu>
>> wrote:
>> > So you're suggesting that GitHub would function as a sort of alternate
>> front-end to Phab. While I've grown to enjoy Phab quite a bit, I still
>> strongly dislike arc, which tries to be too clever for my tastes. Provided
>> the integration works smoothly, I quite like this idea.
>> >
>> > Richard
>> >
>> >> On Sep 27, 2016, at 5:32 PM, Michael Sloan <mgsloan at gmail.com> wrote:
>> >>
>> >> You're welcome Richard!  I look forward to helping make it happen.  In
>> >> the other thread, Alexander Vershilov mentioned that we might instead
>> >> consider the following more straightforward workflow:
>> >>
>> >> 0) Have a bot that watches github for PRs.
>> >> 1) Submit whatever you want to github as a PR.
>> >> 2) It will be automatically closed and migrated to Phabricator.  I
>> >> would like it to automatically create a Phabricator account if you do
>> >> not already have one.  The message from the bot will tell you about
>> >> this action, and explain how to log in, perhaps even linking to
>> >> resources about Phabricator.
>> >>
>> >> Is this worth it?  I think it is for the one-off cases.  However, you
>> >> will have to be prepared that this means that people won't have
>> >> arcanist setup, and therefore are less likely to actually iterate on
>> >> their PR.  Perhaps we should extend this to the following:
>> >>
>> >> 3) Subsequent pushes to the branch for the PR will update the
>> >> Phabricator differential as if you had pushed via Arcanist.  I think
>> >> with this in place, we would have a fully streamlined system that
>> >> allows people to use their familiar GitHub workflows, without needing
>> >> to learn Arcanist.  Interactions would then still occur on , of
>> >> course.
>> >>
>> >> This way, GHC HQ doesn't even need to learn to use this new "ghc-hub"
>> >> tool!  Could name the bot that, though!
>> >>
>> >> Thoughts?  I think it would be great for this to be proposed formally
>> >> soon so that we can make it happen.  I am eager to be able to use my
>> >> normal git workflows, as my little experience with Arcanist induced
>> >> some head-scratching.  Not the fault of the tool, just a result of
>> >> lack of familiarity.
>> >>
>> >> -Michael
>> >>
>> >> On Tue, Sep 27, 2016 at 8:46 AM, Richard Eisenberg <
>> rae at cs.brynmawr.edu> wrote:
>> >>> To sum up, this proposes the following:
>> >>>
>> >>> 1. Allow PRs on GitHub.
>> >>>
>> >>> 2. Michael Sloan to write a new utility, ghc-hub, which automates
>> tasks interfacing between GitHub and Phab. This utility would be used only
>> by GHC HQ and not by contributors.
>> >>>
>> >>> 3. Small GitHub PRs can be merged directly, by ghc-hub.
>> >>>
>> >>> 4. Larger GitHub PRs can be migrated to Phab by ghc-hub. The
>> contributor would be issued a polite email explaining how to set up a Phab
>> account to continue to follow their contribution.
>> >>>
>> >>> Have I captured this accurately? If so, a resounding +1 from me. I’ve
>> wanted exactly this for a while.
>> >>>
>> >>> Is this worth sending through ghc-proposals?
>> >>>
>> >>> Thanks for volunteering item (2), Michael!
>> >>>
>> >>> Richard
>> >>>
>> >>> -=-=-=-=-=-=-=-=-=-=-
>> >>> Richard A. Eisenberg
>> >>> Asst. Prof. of Computer Science
>> >>> Bryn Mawr College
>> >>> Bryn Mawr, PA, USA
>> >>> cs.brynmawr.edu/~rae
>> >>>
>> >>>> On Sep 26, 2016, at 11:09 PM, Manuel M T Chakravarty <
>> chak at justtesting.org> wrote:
>> >>>>
>> >>>> Sounds like a great idea to me and might alleviate SimonM’s concerns
>> about fragmentation of dev attention.
>> >>>>
>> >>>> Manuel
>> >>>>
>> >>>>> Michael Sloan <mgsloan at gmail.com>:
>> >>>>>
>> >>>>> Argh, sent too soon.  The first paragraph, revised:
>> >>>>>
>> >>>>> This sounds like an ideal solution, Ben!  As has been discussed many
>> >>>>> times before, GitHub has many users familiar with its interface.  By
>> >>>>> allowing GitHub PRs, the initial contribution barrier will be
>> lowered. If
>> >>>>> there is an easy and straightforward process for shifting big
>> patches
>> >>>>> to Phabricator, then people who are regularly contributing via
>> GitHub
>> >>>>> PRs can be incrementally on-boarded to the Phabricator / Arcanist
>> >>>>> workflow.
>> >>>>>
>> >>>>> On Mon, Sep 26, 2016 at 12:07 PM, Michael Sloan <mgsloan at gmail.com>
>> wrote:
>> >>>>>> This sounds like an ideal solution, Ben!  As has been discussed
>> many
>> >>>>>> times before, GitHub has many users familiar with its interface.
>> By
>> >>>>>> allowing GitHub PRs, the initial contribution
>> >>>>>>
>> >>>>>> I think it would be acceptable for larger GitHub PRs to have some
>> >>>>>> automated boilerplate response.  Ideally this would look like:
>> >>>>>>
>> >>>>>> """
>> >>>>>> Thanks for making this patch!  I've turned this into a Phab
>> >>>>>> Differential xxx and closed this PR.  Please create a differential
>> >>>>>> account associated with your email address ..."
>> >>>>>> """
>> >>>>>>
>> >>>>>> The email address can be automatically pulled from commit metadata.
>> >>>>>> If one is absent, then this automated process isn't possible.  If
>> it
>> >>>>>> is present and
>> >>>>>>
>> >>>>>> So, I'm imagining a utility that interfaces between both GitHub and
>> >>>>>> Phab,allowing the following commands:
>> >>>>>>
>> >>>>>> * "ghc-hub migrate https://github.com/ghc/ghc/pull/1" - migrates
>> the
>> >>>>>> patch to differential.  It may attempt to migrate body and title of
>> >>>>>> the initial post, but lets not bother with migrating any review
>> data.
>> >>>>>>
>> >>>>>> * "ghc-hub merge https://github.com/ghc/ghc/pull/1" - merges the
>> >>>>>> patch.  This is used for merging small patches.  It would not do an
>> >>>>>> automated push.  Maybe have "--push" also perform the push?  So
>> like
>> >>>>>> if you are on master, then "ghc-hub merge
>> >>>>>> https://github.com/ghc/ghc/pull/1 --push" would merge the patches
>> and
>> >>>>>> push to master.
>> >>>>>>
>> >>>>>> How does this sound?  I like the idea a lot, and would enjoy
>> helping
>> >>>>>> with implementation, time permitting.  I could possibly start
>> hacking
>> >>>>>> on it if others give the go ahead of "Yes, lets do that".
>> >>>>>>
>> >>>>>> -Michael
>> >>>>>>
>> >>>>>> On Mon, Sep 26, 2016 at 11:45 AM, Ben Gamari <ben at smart-cactus.org>
>> wrote:
>> >>>>>>> Carter Schonwald <carter.schonwald at gmail.com> writes:
>> >>>>>>>
>> >>>>>>>> In writing the following huge wall of text, I had and idea that
>> I think
>> >>>>>>>> many folks would find palatable:
>> >>>>>>>>
>> >>>>>>>> What if simple small patches (such as hypothetical drive by doc
>> patches )
>> >>>>>>>> had a mailing list where folks could email the simple / small
>> patches as
>> >>>>>>>> email attachments plus a body text that summarizes the patch,
>> what it does,
>> >>>>>>>> and why it's simple!
>> >>>>>>>>
>> >>>>>>> I completely agree that for small (e.g. documentation) patches our
>> >>>>>>> current system is quite heavy. For this reason I suggested at
>> ICFP that
>> >>>>>>> we simply begin accepting small patches via GitHub pull requests.
>> >>>>>>> Frankly, this is less work for me than merging patches from a
>> mailing
>> >>>>>>> list and I believe many users feel that GitHub is more accessible
>> than a
>> >>>>>>> mailing list.
>> >>>>>>>
>> >>>>>>> The problem of course is what subset of patches do we want to
>> allow to
>> >>>>>>> be taken via GitHub. My suggested answer to that is any patch
>> which, if
>> >>>>>>> I were to write it myself, I would feel comfortable pushing
>> directly to
>> >>>>>>> the tree.
>> >>>>>>>
>> >>>>>>> Then there is the question of what do we do with pull requests
>> opened
>> >>>>>>> which do not satisfy this criterion. In this case I would likely
>> open a
>> >>>>>>> Phabricator Differential with the pull request and close the pull
>> >>>>>>> request with a link to the Diff. In the ideal case this will
>> inspire the
>> >>>>>>> contributor to join the review process on Phabricator; in the
>> worst case
>> >>>>>>> review turns up issues in the patch and the user gives up. Either
>> way, at
>> >>>>>>> least the contributor feels his patch has been seen and given the
>> >>>>>>> attention it deserves.
>> >>>>>>>
>> >>>>>>> Cheers,
>> >>>>>>>
>> >>>>>>> - Ben
>> >>>>>>>
>> >>>>>>> _______________________________________________
>> >>>>>>> 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
>> >>>
>> >
>> _______________________________________________
>> 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/20160929/2e34bb79/attachment.html>

More information about the ghc-devs mailing list