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

[David Greene via llvm-dev]

Emilio Cobos Álvarez <emilio at crisal.io> writes:

> [1] or [2] are recentish examples that come to mind, but it happens
> fairly often. Of course for a bunch of simpler changes one revision is
> enough.

I think you forgot to include links.  :)

> The use cases are similar to the "I have one PR with multiple commits"
> in GitHub, but with the advantage of being able to review them
> individually, and thus they can land upstream sooner.

Downstream we strongly suggest people use a one-commit-per-PR model.
That does incur some additional overhead but I personally haven't found
it to be an issue.  Usually if I have a series of dependent patches,
then I need to merge the first one before I can do the second one, etc.

I can see how having multiple patches up at once for review might speed
things up.  But what if a very first patch in a series needs major
changes and that affects every single following patch?  The series has
to be re-posted anyway, right?  I don't see how this is much better than
reviewing the first patch and getting that merged first, moving on to
the second patch, etc.

If the patches truly are independent, then why not open a separate PR
for each one?  Yes, with GitHub that requires a separate branch for each
but if the changes are truly independent then multiple branches seems
entirely appropriate to me.  IME it helps keep things organized.
Independent changes are independent lines of development.  Changes
needed for one don't affect the state of the others.

If the patches are dependent, then we're back to the situation above
where review causes the need to re-post a new version of the series.  If
review goes smoothly then I guess maybe there is some time saved but how
often does that really happen for any kind of large change that would be
needed to be broken up into multiple commits?

                       -David