@ben cool! yeah... the phab/arc workflow is probably the easiest part of the compiler patch work flow, heck, unlike a lot of projects you get ci for free! :) but thigns that make it easier are always good On Sun, Oct 5, 2014 at 1:03 AM, Ben Gamari wrote:

Richard Eisenberg writes:

...

I've just finished reading this: http://www.reddit.com/r/haskell/comments/2hes8m/the_ghc_source_code_contains...

For better or worse, I don't read reddit often enough to hold a conversation there, so I'll ask my question here: Is there a way we can turn GitHub pull requests into Phab code reviews? I'm thinking of something like this:

...

I'm still quite unsure of how many people exist who,

* find a bug they need to fix, * are willing to dig into the GHC codebase and fix it, * clean up their fix enough to submit upstream, and * take the initiative to send the fix upstream

and yet aren't willing to take the five (twenty?) minutes to familiarize themselves with Phabricator and the arc toolchain.

That being said, I'm all for lowering barriers. I started to write up a quick hack [1] to implement this sort of process. It's a bit late at the moment so I'll just put it up here for comment for the time being. It's a bit messy, the security implications are haven't been considered at all, and half of it is pseudo-code at best. That being said, it's a start and if someone picks it up and finishes it before I wake up tomorrow I won't be offended.

Cheers,

- Ben

[1] https://gist.github.com/bgamari/72020a6186be205d0f33

_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

On 05.10.2014 07:03, Ben Gamari wrote:

and yet aren't willing to take the five (twenty?) minutes to familiarize themselves with Phabricator and the arc toolchain.

Are you serious about this? I think your time estimate is a grand illusion. I attended Joachim Breitner's talk about Phabricator at the GHC developer meeting, that already (nearly?) used up the twenty minutes you allow. Yet I still have to * try it the first time, * make sure I get everything right, * learn to *trust* the tool * that is does the right thing, * does not do anything bad to my files * etc. pp. The brightest might be up to get on track in a couple of hours, but the majority is quite hesitant towards new tools... Human condition. Cheers, Andreas -- Andreas Abel <>< Du bist der geliebte Mensch. Department of Computer Science and Engineering Chalmers and Gothenburg University, Sweden andreas.abel@gu.se http://www2.tcs.ifi.lmu.de/~abel/

Andreas Abel writes:

On 05.10.2014 07:03, Ben Gamari wrote:

...

and yet aren't willing to take the five (twenty?) minutes to familiarize themselves with Phabricator and the arc toolchain.

Are you serious about this? I think your time estimate is a grand illusion.

Fair enough; this may well be an underestimate. To form the number I tried thinking back to my own experience starting off with Phabricator (back in August, IIRC) which went roughly as follows, 1. I asked `thoughtpolice` about this new-fangled Phabricator thing 2. He pointed me to the GHC wiki [1] 3. I ignored nearly everything on the page but `The CLI` section, installing PHP (this is where I'm thankful to be running Linux where package installation is quite straightforward) 4. I ran `arc diff`, 5.a. I reflected on the mild shock of seeing that `arc` had squashed my carefully crafted patch set into a single commit. This still bothers me to this day. 5.b. I moved on with life and had a coffee All-in-all this perhaps took half an hour from start to coffee. Admittedly, I had very little understanding of what was going on underneath the shiny veneer (and more or less still don't), but I did successfully submit a patch. This being said, I can see that there are several places where this can go awry. I hate to think of what this might look like on Windows. Moreover, I have absolutely confidence that git would preserve my work, regardless of what unholy things this new tool did to my repo. Without this confidence I would have tread far more carefully which inevitably would have cost time.

I attended Joachim Breitner's talk about Phabricator at the GHC developer meeting, that already (nearly?) used up the twenty minutes you allow. Yet I still have to

* try it the first time, * make sure I get everything right, * learn to *trust* the tool * that is does the right thing, * does not do anything bad to my files * etc. pp.

The brightest might be up to get on track in a couple of hours, but the majority is quite hesitant towards new tools...

Prior to my experience I had read in a variety of venues about all of the wonderful things that Phabricator would do for us. I understand that casual contributors without this background may find it harder to even motivate beginning the process of picking it up, regardless of how easy this may be.

Human condition.

Point taken. I agree that it can't hurt to expose a more familiar interface to the world. Cheers, - Ben [1] https://ghc.haskell.org/trac/ghc/wiki/Phabricator

This is also the thing that worries most about arc: Squashing commits. Splitting commits into * things that only do whitespace changes * things that only add comments * things that only refactor * things that actually introduce a semantic change *is* very valuable also for the efficency of the review process. On 05.10.2014 19:13, Tuncer Ayaz wrote:

On Sun, Oct 5, 2014 at 4:32 PM, Ben Gamari wrote:

...

5.a. I reflected on the mild shock of seeing that `arc` had squashed my carefully crafted patch set into a single commit. This still bothers me to this day.

I second 5.a, but does it have to be this way, or can arc be instructed to not squash commits?

-- Andreas Abel <>< Du bist der geliebte Mensch. Department of Computer Science and Engineering Chalmers and Gothenburg University, Sweden andreas.abel@gu.se http://www2.tcs.ifi.lmu.de/~abel/

Richard Eisenberg writes:

I absolutely believe that we should use the best tools available and that committed GHC contributors should have to learn these tools as necessary. Though I've had my problems with Phab and `arc`, I'm confident that this tool was chosen after a deliberative process and am grateful that we have leaders in this area in our midst.

Agreed. Phab certainly has a learning curve and is not without its papercuts but on the whole seems to be an excellent tool.

All that said, I think that the suggestion just to accept GitHub pull requests will lead to confusion, if only for the namespace problem. If we start to accept pull requests, then we are de facto going to have to deal with both the GH issue tracker and Trac's (and Phab's), and that is a terrible place to be. Part of the automated response to pull request submissions could be a post on the GH pull request record pointing folks to the Phab review that was created in response. The pull request would then be closed.

This is where I was going with the beginning of a script I posted on Saturday. To me this seems like an excellent compromise: using the familiarity of Github to attract contributions and (hopefully) siphon them into Phabricator. The numbering conflicts may still be problematic but I suspect that in practice people will learn that the Github numbers are meaningless fairly quickly. Cheers, - Ben

To be completely clear, arc does not FORCE you to squash commits. You can simply arc diff each commit in question seperately. Now, it is certainly true that arc does not make this easy to do. See: https://secure.phabricator.com/T5636 Edward Excerpts from Andreas Abel's message of 2014-10-06 03:27:10 -0600:

This is also the thing that worries most about arc: Squashing commits. Splitting commits into

* things that only do whitespace changes * things that only add comments * things that only refactor * things that actually introduce a semantic change

*is* very valuable also for the efficency of the review process.

On 05.10.2014 19:13, Tuncer Ayaz wrote:

...

On Sun, Oct 5, 2014 at 4:32 PM, Ben Gamari wrote:

...

5.a. I reflected on the mild shock of seeing that `arc` had squashed my carefully crafted patch set into a single commit. This still bothers me to this day.

I second 5.a, but does it have to be this way, or can arc be instructed to not squash commits?

Herbert Valerio Riedel writes:

On 2014-10-05 at 04:51:41 +0200, Richard Eisenberg wrote:

...

I've just finished reading this: http://www.reddit.com/r/haskell/comments/2hes8m/the_ghc_source_code_contains...

For better or worse, I don't read reddit often enough to hold a conversation there, so I'll ask my question here: Is there a way we can turn GitHub pull requests into Phab code reviews? I'm thinking of something like this:

My greatest worry about allowing GitHub PRs to the github.com.ghc/ghc.git repo is that GitHub and Trac use the very same `#[0-9]+` syntax tokens for referring to tickets and PRs, and trigger actions as soon as they detect any commit using that token. In other words, there's a namespace collisions (Luckily, Phabricator seems to have been designed to be used in concert with an external Ticket tracker, so it uses `[DT][0-9]+` to refer to code-revisions & tickets respectively).

Morever, I'm also worrying this may become confusing to new contributors, since we already have the Trav vs Phabricator confusion about where to submit patches; if we also add GitHub PRs it'll just add another item to be confused about where things ought to be submitted. And the more PRs are added on github.com/ghc/ghc, the more it may appear as if that is the encouraged way to submit them (even though Phabricator+Trac is our currently targetted workflow)

What I'd suggest alternatively, since this what some of our contributors are already doing instead of uploading patches:

Teach Phabricator to allow to submit a URL to a commit (or branch) in a forked github.com/ghc/ghc repo, and create a code-revision out of that.

This is a nice idea and sounds simple to implement. It certainly doesn't reduce the contributor-side friction nearly as well as accepting pull requests but it may be good enough. It would be quite straightforward to adapt the code I hacked together last night into such an interface. Cheers, - Ben

On Sun, Oct 5, 2014 at 12:56 PM, Joachim Breitner wrote:

Hi,

we already have the problem with some people submitting diffs on trac, other submitting git patches on trac or linking to their private fork somewhere to pull from, and others using Phabricator. And, at least as far as I can tell from here, it doesn’t seem to be a big deal.

So (warning: surprisingly simple solution ahead) we could consider the option of simply accepting Github pull requests!

I think it could work well ok if we either * somehow communicate that people should open a trac ticket as well, if they want to make sure their contribution is handled in a timely manner, or (or and) * someone of us looks after PR and creates trac tickets as needed.

The advantage is clear: Low entry barrier for new contributors and, as Michael says, for very small contributions (documentation typo fixes or so).

The downside is that we have more tools to work with. This either means that we all get to know the GitHub frontend (which is quite intuitive, and at least reading diffs and commenting should be possible for all without a lot of learning), or that we simply let those developers who are familiar with GitHub handle it.

I think the advantage could outweigh the downside and it’s worth a try. We don’t even have to advocate it aggressively, just remove the „Do not submit PRs“ notice on the repo and see what happens.

(The problem with the ticket numbers remain, unfortunately.)

There's also the problem that Github's review system is not as powerful and most importantly does not preserve history like Gerrit or Phabricator do. Once used to it, maintainers probably won't be happy to lose productivity due to the simplistic review system.

Is there any particular reason why taking in GitHub pull requests would be more problematic than, say, applying patches attached to Trac bugs? Both have to be dealt with manually by someone with commit rights for the canonical repository anyway. If the issue is important enough that, say, tracking and reviews come into play, the contributor could always be asked to move the patch to Phabricator/Trac. Let's keep the barriers as low as possible...

Hi, Am Montag, den 06.10.2014, 17:54 +0200 schrieb Tuncer Ayaz:

By the way, while the Github team has no public ticket system, they are very responsive when you send them feature requests or, say, explain where the review system is incomplete/broken. They never promise anything and do not pre-announce a feature, so it is hard to track future changes. However, they're responsive and seem to value majority opinion.

Here's the page: https://github.com/support

good idea, I sent them a message which is basically http://stackoverflow.com/questions/26204811/prevent-github-from-interpreting... Greetings, Joachim -- Joachim “nomeata” Breitner mail@joachim-breitner.de • http://www.joachim-breitner.de/ Jabber: nomeata@joachim-breitner.de • GPG-Key: 0xF0FBF51F Debian Developer: nomeata@debian.org

On Tue, Oct 7, 2014 at 8:42 PM, Joachim Breitner wrote:

doesn't look like that will happen soon:

...

Hey Joachim,

Disabling that linking is not possible currently, and I'm not sure if that feature will be available in the near future. Still, I'll add your request to our feature request wishlist and pass the feedback to the team.

Thanks for the question/suggestion and let us know if there's anything else.

That's unsurprising, given how that linking scheme is used everywhere, but they responded quickly. I've sent them suggestions on improving the review system, and they have acknowledged working on that, but it's a behind closed doors development process without any pre-announcement or commitment. Another popular request is for projects like GHC or similar (that have their own or different infrastructure for discussion and contributions) to want to disable Pull-Requests (like Wiki or Issues). They say it's often requested but have no immediate plans to add a check box. It's unfortunate because you have to constantly close pull requests with a link to the contributing guide.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 20/10/14 21:13, Ben Gamari wrote:
> 
>  1. Do nothing, ignore pull requests as we do now
> 
>  2. Monitor Github for new pull requests and close with a message
>     requesting that the user opens a differential instead
This is stupid. Instead, disable pull requests on GitHub altogether.

>  3. Teach Phabricator to allow to submit a URL to a commit (or branch)
>     in a forked github.com/ghc/ghc repo, and create a code-revision out
>     of that. (suggested by hvr)
>     
>  4. Monitor Github for new pull requests and use facility in (3) to 
>     open a differrential and close the pull request with a message
>     pointing to it.
If someone can be bothered implementing these, they could work.

>  5. Start accepting pull requests in addition to differentials
>     (suggested by Joachim)
- -1.
- -- 
Alexander
alexander@plaimi.net
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRGCKsACgkQRtClrXBQc7U5jgD+NFGH9wNB8t54K8v2xjOQ3U1a
T4W6hPkKbmUNrdPjf2YA/0eDUG2XJgEvxiuSnVtmimLgrGFc2weD9f1656/S2tm8
=3ELn
-----END PGP SIGNATURE-----