
On 26 September 2016 at 20:13, Ben Gamari
Simon Marlow
writes: I would rather we *didn't* accept contributions via github, even for small patches, and instead put more effort into streamlining the Phabricator workflow.
- Adding another input method complicates the workflow, users have to decide which one to use
I think we would want to try to sell the GitHub route as "if you would like to contribute then we would strongly prefer you use Phabricator, but if you must and it's a small patch, we will accept it via GitHub."
But this is opening the floodgates a crack... how do we know what a "small" patch is? What happens when someone submits a patch that's too large? The patches will get larger, we'll have to do code reviews on two different tools, and it will be really hard to go back. I just have a bad feeling about this.
- Github is not integrated with our other infrastructure, while
Phabricator is
True, but I suspect for the small documentation patches that we are currently consider this shouldn't matter so much.
- Mutliple sources of contributions makes life harder for maintainers
It does certainly put yet another task on our plates, but I would argue that it's actually easier than accepting patches via Trac, which we already do.
We should stop accepting patches via Trac too :) Cheers Simon
- I also like the idea of auto-push if validate succeeds. Or a button that you can press on the diff that would do the same thing, so you can get code review first.
To be clear, I'm a bit weary of opening up the auto-push feature to new contributors. While regular contributors know what changes can be safely pushed and which require review, we have no guarantee that a new contributor has developed these sensibilities.
- +1 to making the manual easier to build. The same goes for Haddocks; it's really hard to make a simple patch to the docs and test it right now.
The users guide should be quite possible to do.
I don't believe there is any reliable way to allow a contributor to build the haddocks without having built GHC (since you need GHC master to parse `base`, et al.); that being said, we could have Harbormaster upload built documentation somewhere and then leave a link to it on the Diff.
One other thing that came up but wasn't mentioned in the notes: let's be more prompt about reverting patches that break validate, even if they only break a test. Now that we have better CI support, we can easily identify breaking patches and revert them.
Agreed.
Cheers,
- Ben