Proposal: accept pull requests on GitHub

[Tuncer Ayaz]

On Thu, Sep 3, 2015 at 6:41 AM, Austin Seipp <austin at well-typed.com> wrote:
> (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.

Let me start off by saying I'm not arguing for GitHub or anything else
to replace Phabricator. I'm merely trying to understand the problems
with merge commits and patch sets.

>   - 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'm genuinely curious about the need to avoid merge commits. I do
avoid merge-master-to-topic-branch commits in submitted diffs, but
unless you always only merge a single cumulative commit for each diff,
merge commits are very useful for vcs history.

>   - 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.

I would also like to understand why reviewing a single commit is
easier than the steps (commits) that led to the whole diff. Maybe I
review stuff differently, but, as I wrote yesterday, I've always found
it easier to follow the changes when it's split into proper commits.
And instead of "big patch" I should have written "non-trivial patch".
A 100-line unified diff can be equally hard to follow as a 1000-line
diff, unless each diff hunk is accompanied with code comments. But
comments don't always make sense in the code, and often enough it's
best to keep it in the commit message only. Hence the need for
splitting the work, and ideally committing as you work on it, with a
final cleanup of rearranging commits into a proper set of commits. I'm
repeating myself, but git-bisect is much more precise with relevant
changes split up as they happened.