[llvm-dev] [cfe-dev] Phabricator -> GitHub PRs?

[David Greene via llvm-dev]

David Blaikie <dblaikie at gmail.com> writes:

>> I can see that too.  For commits that are dependent on a long series of
>> other commits (D48017 for example), how hard is it to review without
>> reviewers also looking at the commits upon which it depends?
>
> Sometimes easy, sometimes harder - and depends on the kind of review. for
> me this really helps keep the substantive parts of the review focussed -
> you can move depedent refactoring into separate reviews that are easy to
> eyeball knowing you aren't expecting to find anything "interesting". Then
> separately review the "interesting" stuff without all the noise of the
> refactoring. That's super important to me.

Yes, I can definitely appreciate that.

>>   Do people really review such commits early in the process?  Even
>> without the early review I agree the context can be useful.
>
> Yep - I might do superficial review early - basic stylistic stuff to get
> that out of the way even if the substance isn't ready to be reivewed yet
> (most often by suggesting ways to pull out refactorings to focus the main
> review on the substantive parts).

Ok.  If it is true that GitHub can be configured to show a branch as a
series of diffs rather than a giant diff, hopefully we'd have similar
functionality.  Of course someone would need to verify that.  :)

>From what I have read GitHub does *not* present comments attached to
individual commit diffs, which is certainly a downgrade from Phab.

>> I'm not sure how this differs from having multiple patches up for review
>> simultaneously.  I do that all the time and is just as easy (for me)
>> with GitHub PRs.  Are you simply saying that you can have multiple patch
>> series up for review at the same time?  I feel like I am not grasping
>> some (to me) subtle point you're making.
>
> I believe he's saying that having patch series grouped together makes it
> easier to think of it as a logical unit. Rather than "I have 10 patches out
> for review" thinking of it as "I have 5 features out for review"  -maybe
> one complicated on ein the form of a sries of 5 commits, and a small pair,
> and 4 singles, for instance.

Makes sense, that's what I assumed.  I am not sure Phab provides any
benefit over GitHub PRs in this regard.