Hello, everyone!
As a newbie to LLVM I find using Phabricator less comfortable and more intimidating than Github Pull Requests. Also, committing to the main branch directly feels off.
So, I’m just wondering, is there any plans for moving to Github PRs?
I think I’d encourage more people to contribute, because it’s easier to create a PR than creating a patch via Phabricator if you never contributed to LLVM before.
This has been discussed a lot - short answer is probably no for the near term. Many contributors feel that phabricator provides a better review tool and that GitHub lacks important features. Others feel like GitHub would be more inclusive and easier for beginners and that those features are not as important for them.
There is a long thread here: A Request for Comment on Code Review Process · Issue #73 · llvm/llvm-iwg · GitHub
Phab can be intimidating at the start - but feel free to ask questions if you run into problems. Many will be happy to help guide you through.
Thank you for the answer! I’m going to read that thread
Could you please say which features of Phabricator you find more useful for your workflow that are absent in Github?
I think You can find all the discussion/opinion about the same , here itself A Request for Comment on Code Review Process · Issue #73 · llvm/llvm-iwg · GitHub .
I am probably not the best person to ask. I favour a move to GitHub. But the thing that many seem to point out is how bad GH is at handling when you force push a PR. Then comments made on the previous code is no longer visible. Another one is how you can “stack” commits and reviews to create dependencies between them.
I have been using GitHub for a long time and Phab for less time - but personally I don’t think these features are more worth than lowering the barrier to entry. But I am just one voice.
Could you please say which features of Phabricator you find more useful for your workflow that are absent in Github?
To summarize the general gist I’ve gotten from many, many discussions on the issue:
You also missed the lack of availability for Herald (even though we have some ideas about how to replace it with some custom actions maybe).
For features (like stacked review) it might also be thing that experienced contributors leverage while newcomers won’t see the kind of things that Phab supports and GH struggles with.
Anyway, IMO it may be less of « if » but « when » and « how » are we gonna switch (that is that what we need is a comprehensive draft for a proposal to use PRs which include all the existing workflows, with examples of scenarios and how they are handled / displayed in GH).
One wrinkle is that we don’t have a successful collaboration with GH to understand or influence their roadmap…
Stacked reviews (i.e. linked PRs) are supported in GH. What specifically do you see as missing in how GH does it?
There are some high-traffic GitHub repositories with issues and PRs. I believe that it can be a very agile workflow.
LLVM can always design custom GitHub Apps.
@kparzysz-quic the problem is to design a consistent workflow. If you take every single feature individually you can “make it work” with pull-requests, but each comes with specific requirements and often these a contradicting.
For example, for linked PRs to work you need to push branches to the same repo as the target repo, you can do this from a fork (but we don’t want to have all these branches in the repo!). Also, how do you manage updates to your PRs? A straightforward way to replicate what we can do in Phab is to have N commits in sequence locally and push each of them to a separate branch on the remote (and then cascade the linked PRs). When updating you likely gonna have to rebase and force-push, however on other part of the thread linked above, it was mentioned that for GitHub to correctly and consistently show comments across review iterations (and also for having the ability to see updates from one review to the next) you need to append new commits and never rebase/force-push.
I guess the only way to have linked PRs is to have locally separate branches, that never rebase, and instead run locally N cascading merges for every “rebase” (that is what I do with a single rebase today).
(All of this is in the thread on the issue linked above I think, admittedly it is very very long)
So you have the workflow, exactly as you described: you merge the branches instead of rebasing. I believe that when you “squish and merge”, the merge commits go away. As for having local branches, you can turn on autodeletion of merged branches to reduce the clutter, although having that with forks would be nice [1].
One thing to keep in mind is that GitHub is a repo, whereas Phab is just a diff&comment tool. What you do on GitHub has to reflect the actual state of the underlying repo, and that implies certain restrictions. You can make it consistent, but it won’t necessarily look exactly like the prior Phab workflow.
That same thread you mention also showed that there is a lot of either misunderstanding of GH, or unfamiliarity with it. I have a feeling that that in itself factors into the resistance, not just any potential technical reasons.
[1] One thing I haven’t tried is having the first PR go from fork to the main repo, then the linked ones be entirely inside the fork: after merging the first PR, would the next one auto-rebase into main, and what would happen to the second PR?
Right, I find this completely unappealing compared to the simplicity of the linear rebase and amend workflow I have available right now.
Which is likely why GitHub is technically inferior by forcing trade off that we don’t lock ourselves in with Phab right now.
This isn’t as much about the “rebase automatically” it is that the linked ones would never show up in the main repo for review, they would be pull request in the fork only (the PR ID and the URL would be local to the fork in the first place)
You’re underestimating the experience of many folks (including myself) who are actively using GitHub on other projects and reported in this thread many aspects in how we feel much less productive than with what we have on Phab. It is not “just different” (to begin with, GitHub offers no good replacement for Herald still?)
I think this thread is now rehashing a lot of things in the GitHub issue… I am not sure how productive that is. I think we need to wait for a more organised next step from IWG or some other initiative to work on an exact workflow if we where to move.
Whether Phab is technically superior is a matter of debate, and is actually irrelevant. A bunch of technically superior components do not make a package, let alone a technically superior package. GH is integrated with the repo, and what matters is whether the PR mechanism is good enough. What we have now is a set of disjoint tools held together with some equivalent of duct tape, e.g. llvm/docs/HowDoIEven.rst.
What my experience tells me is that every change takes some effort to get used to. The transition period is often unpleasant and less productive, but that goes away after a while—that some things take a few more steps no longer makes a difference.
Right, and many folks (with significant experience both with GitHub PR and with Phabricator) explained that it isn’t “good enough” in their experience, or to say differently: this comes with noticeable decrease of productivity which may or may not be “good enough” depending on how much you care about some experienced individual contributors productivity in the project.
Again as I mentioned before in this thread, my opinion is that it is not “if” but “when” and “how” are we gonna migrate, and also that what’s actually needed to move forward IMO is a comprehensive workflow proposal to move LLVM to GitHub PR that includes a detailed description (with examples) of the before/after for the existing workflows. Of course that requires involvement from folks who actually understand the existing workflow and their subtleties as well as the subtleties associated with GitHub.
I’m still waiting for any sort of metric quantifying these “experiences” people have that want us to change to their favorite “tool”. For all I know, we loose people and engagement every time we force a change on the community. That’s at least my “experience”.
I’m always a fan of data so I agree that metrics would be useful. Regarding losing people when changing tools, I’ve observed that on another project so it’s a valid concern, but I’ve also seen the reverse: more contributors joining a project when it switches to a more common tool. I’d be glad to share offline some experiences in which changing tools made a big difference in attracting developers. Phabricator is at least more tolerable than some other practices I’ve observed on a large project, but that’s partly because I’m fortunate to have a pair programming partner who reminds me of the steps to submit or update a diff in Phabricator every time I submit one. By contrast, I have no trouble remembering how to submit a GitHub PR in most circumstances.