[llvm-dev] Phabricator -> GitHub PRs?

Renato Golin via llvm-dev llvm-dev at lists.llvm.org
Thu Jan 16 11:24:38 PST 2020


On Thu, 16 Jan 2020 at 19:10, David Blaikie <dblaikie at gmail.com> wrote:
> Can you point to examples of that - where Phab links have been used to express non-mechanically-dependent patches?

Not at the top of my head, but since that's not what we're talking
about, I'll go to the next point.

> Approval order isn't commit order - I'm more than happy to approve a later patch in a series, even if the review of an earlier patch necessitates some rework in a later patch - such as renaming a function. The later patch I already approved must be updated to use the new name of the function, but I'm fine with that, same as I would be if it'd been an independent change that did the rename in the time it took us to review that patch.

I see what you mean. Once all are approved, the commit happens, but
they can be approved in any order.

I used to do that too, but I honestly don't like it. Earlier patches
can significantly change the following changes, then the approval has
to be reverted.

Nowadays, on both Phab and Git I just say "LGTM once the others are
approved as is" and once I approve the earlier ones, I scan the
remaining series and approve them all.

I think the process of uploading multiple commits in separate and then
creating a link from each one to the next is cumbersome and error
prone (to me particularly, I make many mistakes).

> What do you mean by "with mention" and what do you mean by "related" (I guess you man "not dependent, but interesting to consider together")

When you write a ticket number, either in phab (D12345) or GitHub
(#123), the UI creates a link to the mentioned issue/review. In
Github, if you say things like "fixes #123", it closes automatically
(which is not necessarily right, like "partially fixes #123" :).


> My understanding from other people's comments (I've not tried it myself) is that updating a patch series in github is problematic - that it sends new/separate review email rather than being able to associate each patch in the series with ongoing review feedback and updates?

Both Phab and GitHub are problematic in different ways. In Phab, an
earlier change that trickles to the rest of the series needs to be
updated on all patches. In GitHub, some information is lost,
especially if it's a force push, but it only sends one email for the
series.

> Perhaps there's some misunderstanding/different experiences there (as there is with Phab). Maybe you could make a little example? Showing how a dependent patch series with ongoing review feedback works with github PRs?

There's probably a better such example on the internet. I ended up
dragging myself into this corner, but I'm not a defender of GitHub
PRs. I just dislike Phab more than it. ;)

--renato


More information about the llvm-dev mailing list