Proposal: accept pull requests on GitHub

[Greg Weber]

> (I'm tempted naively to ask: is there an automated way to go from a
GitHub PR to a Phab ticket?  Then we could convert the former (if someone
wants to submit that way) into the latter.)

yes, a github PR is just a branch. Thanks for bringing the discussion back
on track to a productive approach.
This suggestion is what some of us were getting at and it would be better
to just limit the discussion to this idea.

On Mon, Sep 7, 2015 at 6:47 AM, Simon Peyton Jones <simonpj at microsoft.com>
wrote:

> I am very much at the ignorant end of this debate: I'll just use whatever
> I'm told to use.  But I do resonate with this observation from Austin:
>
> |  For one, having two code review tools of any form is completely
> |  bonkers, TBQH. This is my biggest 'obvious' blocker. If we're going to
> |  switch, we should just switch. Having to have people decide how to
> |  contribute with two tools is as crazy as having two VCSs and just a
> |  way of asking people to get *more* confused, and have us answer more
> |  questions. That's something we need to avoid.
>
> As a code contributor and reviewer, this is awkward. As a contributor, how
> do I choose? As a reviewer I'm presumably forced to learn both tools.
>
> But I'll go with the flow... I do not have a well-informed opinion about
> the tradeoffs.
>
> (I'm tempted naively to ask: is there an automated way to go from a GitHub
> PR to a Phab ticket?  Then we could convert the former (if someone wants to
> submit that way) into the latter.)
>
> Simon
>
>
> |  -----Original Message-----
> |  From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of
> |  Austin Seipp
> |  Sent: 03 September 2015 05:42
> |  To: Niklas Hambüchen
> |  Cc: Simon Marlow; ghc-devs at haskell.org
> |  Subject: Re: Proposal: accept pull requests on GitHub
> |
> |  (JFYI: I hate to announce my return with a giant novel of negative-
> |  nancy-ness about a proposal that just came up. I'm sorry about this!)
> |
> |  TL;DR: I'm strongly -1 on this, because I think it introduces a lot of
> |  associated costs for everyone, the benefits aren't really clear, and I
> |  think it obscures the real core issue about "how do we get more
> |  contributors" and how to make that happen. Needless to say, GitHub
> |  does not magically solve both of these AFAICS.
> |
> |  As is probably already widely known, I'm fairly against GitHub because
> |  I think at best its tools are mediocre and inappropriate for GHC - but
> |  I also don't think this proposal or the alternatives stemming from it
> |  are very good, and that it reduces visibility of the real, core
> |  complaints about what is wrong. Some of those problems may be with
> |  Phabricator, but it's hard to sort the wheat from the chaff, so to
> |  speak.
> |
> |  For one, having two code review tools of any form is completely
> |  bonkers, TBQH. This is my biggest 'obvious' blocker. If we're going to
> |  switch, we should just switch. Having to have people decide how to
> |  contribute with two tools is as crazy as having two VCSs and just a
> |  way of asking people to get *more* confused, and have us answer more
> |  questions. That's something we need to avoid.
> |
> |  For the same reason, I'm also not a fan of 'use third party thing to
> |  augment other thing to remove its deficiencies making it OK', because
> |  the problem is _it adds surface area_ and other problems in other
> |  cases. It is a solution that should be considered a last resort,
> |  because it is a logical solution that applies to everything. If we
> |  have a bot that moves GH PRs into Phab and then review them there, the
> |  surface area of what we have to maintain and explain has suddenly
> |  exploded: because now instead of 1 thing we have 3 things (GH, Phab,
> |  bot) and the 3 interactions between them, for a multiplier of *six*
> |  things we have to deal with. And then we use reviewable,io, because GH
> |  reviews are terrible, adding a 4th mechanism? It's rube goldberg-ian.
> |  We can logically 'automate' everything in all ways to make all
> |  contributors happy, but there's a real *cognitive* overhead to this
> |  and humans don't scale as well as computers do. It is not truly
> |  'automated away' if the cognitive burden is still there.
> |
> |  I also find it extremely strange to tell people "By the way, this
> |  method in which you've contributed, as was requested by community
> |  members, is actually a complete proxy for the real method of
> |  contributing, you can find all your imported code here". How is this
> |  supposed to make contribution *easier* as opposed to just more
> |  confusing? Now you've got the impression you're using "the real thing"
> |  when in reality it's shoved off somewhere else to have the nitpicking
> |  done. Just using Phabricator would be less complicated, IMO, and much
> |  more direct.
> |
> |  The same thing goes for reviewable.io. Adding it as a layer over
> |  GitHub just makes the surface area larger, and puts less under our
> |  control. And is it going to exist in the same form in 2 or 3 years?
> |  Will it continue to offer the same tools, the same workflows that we
> |  "like", and what happens when we hit a wall? It's easy to say
> |  "probably" or "sure" to all this, until we hit something we dislike
> |  and have no possibility of fixing.
> |
> |  And once you do all this, BTW, you can 'never go back'. It seems so
> |  easy to just say 'submit pull requests' once and nothing else, right?
> |  Wrong. Once you commit to that infrastructure, it is *there* and
> |  simply taking it out from under the feet of those using it is not only
> |  unfortunate, it is *a huge timesink to undo it all*. Which amounts to
> |  it never happening. Oh, but you can import everything elsewhere! The
> |  problem is you *can't* import everything, but more importantly you
> |  can't *import my memories in another way*, so it's a huge blow to
> |  contributors to ask them about these mental time sinks, then to forget
> |  them all. And as your project grows, this becomes more of a memory as
> |  you made a first and last choice to begin with.
> |
> |  Phabricator was 'lucky' here because it had the gateway into being the
> |  first review tool for us. But that wasn't because it was *better* than
> |  GitHub. It was because we were already using it, and it did not
> |  interact badly with our other tools or force us to compromise things -
> |  so the *cost* was low. The cost is immeasurably higher by default
> |  against GitHub because of this, at least to me. That's just how it is
> |  sometimes.
> |
> |  Keep in mind there is a cost to everything and how you fix it. GitHub
> |  is not a simple patch to add a GHC feature. It is a question that
> |  fundamentally concerns itself with the future of the project for a
> |  long time. The costs must be analyzed more aggressively. Again,
> |  Phabricator had 'first child' preferential treatment. That's not
> |  something we can undo now.
> |
> |  I know this sounds like a lot of ad hoc mumbo jumbo, but please bear
> |  with me: we need to identify the *root issue* here to fix it.
> |  Otherwise we will pay for the costs of an improper fix for a long
> |  time, and we are going to keep having this conversation over, and over
> |  again. And we need to weigh in the cost of fixing it, which is why I
> |  mention that so much.
> |
> |  So with all this in mind, you're back to just using GitHub. But again
> |  GitHub is quite mediocre at best. So what is the point of all this?
> |  It's hinted at here:
> |
> |  > 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).
> |
> |  The real hint is that "the number of contributions will go up". That's
> |  a noble goal and I think it's at the heart of this proposal.
> |
> |  Here's the meat of it question: what is the cost of achieving this
> |  goal? That is, what amount of work is sufficient to make this goal
> |  realizable, and finally - why is GitHub *the best use of our time for
> |  achieving this?* That's one aspect of the cost - that it's the best
> |  use of the time. I feel like this is fundamentally why I always seem
> |  to never 'get' this argument, and I'm sure it's very frustrating on
> |  behalf of the people who have talked to me about it and like GitHub.
> |  But I feel like I've never gotten a straight answer for GHC.
> |
> |  If the goal is actually "make more people contribute", that's pretty
> |  broad. I can make that very easy: give everyone who ever submits a
> |  patch push access. This is a legitimate way to run large projects that
> |  has worked. People will almost certainly be more willing to commit,
> |  especially when overhead on patch submission is reduced so much. Why
> |  not just do that instead? It's not like we even mandate code review,
> |  although we could. You could reasonably trust CI to catch and revert
> |  things a lot of the time for people who commit directly to master. We
> |  all do it sometimes.
> |
> |  I'm being serious about this. I can start doing that tomorrow because
> |  the *cost is low*, both now and reasonably speaking into some
> |  foreseeable future. It is one of many solutions to raw heart of the
> |  proposal. GitHub is not a low cost move, but also, it is a *long term
> |  cost* because of the technical deficiencies it won't aim to address
> |  (merge commits are ugly, branch reviews are weak, ticket/PR namespace
> |  overlaps with Trac, etc etc) or that we'll have to work around.
> |
> |  That means that if we want GitHub to fix the "give us more
> |  contributors" problem, and it has a high cost, it not only has _to fix
> |  the problem_, it also has to do that well enough to offset its cost. I
> |  don't think it's clear that is the case right now, among a lot of
> |  other solutions.
> |
> |  I don't think the root issue is "We _need_ GitHub to get more
> |  contributors". It sounds like the complaint is more "I don't like how
> |  Phabricator works right now". That's an important distinction, because
> |  the latter is not only more specific, it's more actionable:
> |
> |    - Things like Arcanist can be tracked as a Git submodule. There is
> |  little to no pain in this, it's low cost, and it can always be
> |  synchronized with Phabricator. This eliminates the "Must clone
> |  arcanist" and "need to upgrade arcanist" points.
> |
> |    - Similarly when Phabricator sometimes kills a lot of builds, it's
> |  because I do an upgrade. That's mostly an error on my part and I can
> |  simply schedule upgrades regularly, barring hotfixes or somesuch. That
> |  should basically eliminate these. The other build issues are from
> |  picking the wrong base commit from the revision, I think, which I
> |  believe should be fixable upstream (I need to get a solid example of
> |  one that isn't a mega ultra patch.)
> |
> |    - If Harbormaster is not building dependent patches as mentioned in
> |  WhyNotPhabricator, that is a bug, and I have not been aware of it.
> |  Please make me aware of it so I can file bugs! I seriously don't look
> |  at _every_ patch, I need to know this. That could have probably been
> |  fixed ASAP otherwise.
> |
> |    - We can get rid of the awkwardness of squashes etc by using
> |  Phabricator's "immutable" history, although it introduces merge
> |  commits. Whether this is acceptable is up to debate (I dislike merge
> |  commits, but could live with it).
> |
> |    - I do not understand point #3, about answering questions. Here's
> |  the reality: every single one of those cases is *almost always an
> |  error*. That's not a joke. Forgetting to commit a file, amending
> |  changes in the working tree, and specifying a reviewer are all total
> |  errors as it stands today. Why is this a minus? It catches a useful
> |  class of 'interaction bugs'. If it's because sometimes Phabricator
> |  yells about build arifacts in the tree, those should be .gitignore'd.
> |  If it's because you have to 'git stash' sometimes, this is fairly
> |  trivial IMO. Finally, specifying reviewers IS inconvenient, but
> |  currently needed. We could easily assign a '#reviewers' tag that would
> |  add default reviewers.
> |      - In the future, Phabricator will hopefully be able to
> |  automatically assign the right reviewers to every single incoming
> |  patch, based on the source file paths in the tree, using the Owners
> |  tool. Technically, we could do that today if we wanted, it's just a
> |  little more effort to add more Herald rules. This will be far, far
> |  more robust than anything GitHub can offer, and eliminates point #3.
> |
> |    - Styling, linting etc errors being included, because reviews are
> |  hard to create: This is tangential IMO. We need to just bite the
> |  bullet on this and settle on some lint and coding styles, and apply
> |  them to the tree uniformly. The reality is *nobody ever does style
> |  changes on their own*, and they are always accompanied by a diff, and
> |  they always have to redo the work of pulling them out, Phab or not.
> |  Literally 99% of the time we ask for this, it happens this way.
> |  Perhaps instead we should just eliminate this class of work by just
> |  running linters over all of the source code at once, and being happy
> |  with it.
> |
> |    Doing this in fact has other benefits: like `arc lint` will always
> |  _correctly_ report when linting errors are violated. And we can reject
> |  patches that violate them, because they will always be accurate.
> |
> |    - As for some of the quotes, some of them are funny, but the real
> |  message lies in the context. :) In particular, there have been several
> |  cases (such as the DWARF work) where the idea was "write 30 commits
> |  and put them on Phabricator". News flash: *this is bad*, no matter
> |  whether you're using Phabricator or not, because it makes reviewing
> |  the whole thing immensely difficult from a reviewer perspective. The
> |  point here is that we can clear this up by being more communicative
> |  about what we expect of authors of large patches, and communicating
> |  your intent ASAP so we can get patches in as fast as possible. Writing
> |  a patch is the easiest part of the work.
> |
> |  And more:
> |
> |    - Clean up the documentation, it's a mess. It feels nice that
> |  everything has clear, lucid explanations on the wiki, but the wiki is
> |  ridiculously massive and we have a tendancy for 'link creep' where we
> |  spread things out. The contributors docs could probably stand to be
> |  streamlined. We would have to do this anyway, moving to GitHub or not.
> |
> |    - Improve the homepage, directly linking to this aforementioned
> |  page.
> |
> |    - Make it clear what we expect of contributors. I feel like a lot of
> |  this could be explained by having a 5 minute drive-by guide for
> |  patches, and then a longer 10-minute guide about A) How to style
> |  things, B) How to format your patches if you're going to contribute
> |  regularly, C) Why it is this way, and D) finally links to all the
> |  other things you need to know. People going into Phabricator expecting
> |  it to behave like GitHub is a problem (more a cultural problem IMO but
> |  that's another story), and if this can't be directly fixed, the best
> |  thing to do is make it clear why it isn't.
> |
> |  Those are just some of the things OTTOMH, but this email is already
> |  way too long. This is what I mean though: fixing most of these is
> |  going to have *seriously smaller cost* than moving to GitHub. It does
> |  not account for "The GitHub factor" of people contributing "just
> |  because it's on GitHub", but again, that value has to outweigh the
> |  other costs. I'm not seriously convinced it does.
> |
> |  I know it's work to fix these things. But GitHub doesn't really
> |  magically make a lot of our needs go away, and it's not going to
> |  magically fix things like style or lint errors, the fact Travis-CI is
> |  still pretty insufficient for us in the long term (and Harbormaster is
> |  faster, on our own hardware, too), or that it will cause needlessly
> |  higher amounts of spam through Trac and GitHub itself. I don't think
> |  settling on it as - what seems to be - a first resort, is a really
> |  good idea.
> |
> |
> |  On Wed, Sep 2, 2015 at 4:09 PM, Niklas Hambüchen <mail at nh2.me> wrote:
> |  > On 02/09/15 22:42, Kosyrev Serge wrote:
> |  >> As a wild idea -- did anyone look at /Gitlab/ instead?
> |  >
> |  > Hi, yes. It does not currently have a sufficient review
> |  functionality
> |  > (cannot handle multiple revisions easily).
> |  >
> |  > On 02/09/15 20:51, Simon Marlow wrote:
> |  >> It might feel better
> |  >> for the author, but discovering what changed between two branches
> |  of
> |  >> multiple commits on github is almost impossible.
> |  >
> |  > I disagree with the first part of this: When the UI of the review
> |  tool
> |  > is good, it is easy to follow. But there's no open-source
> |  > implementation of that around.
> |  >
> |  > I agree that it is not easy to follow on Github.
> |  > _______________________________________________
> |  > ghc-devs mailing list
> |  > ghc-devs at haskell.org
> |  > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
> |  >
> |
> |
> |
> |  --
> |  Regards,
> |
> |  Austin Seipp, Haskell Consultant
> |  Well-Typed LLP, http://www.well-typed.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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20150908/a392d9f8/attachment-0001.html>