4 ms·

I only tried out Phabricator briefly, and it was pretty good. But the data model bothered me in how it introduces its own abstraction of patches on top of the g

I only tried out Phabricator briefly, and it was pretty good. But the data model bothered me in how it introduces its own abstraction of patches on top of the git branch model. It seems redundant, and also seems like an SVN-inspired model that just felt like a step backwards from having the power of plain git (as in the ability to create diff objects and branch and rearrange them arbitrarily in a pretty simple way, and have multiple people working on these branches at the same time and keeping in sync with each other). I know you can still use plain git locally with Phabricator, but the extra layer of abstraction seemed to only add complexity since now you basically have different data models locally as you do on the server whereas previously they were the same. And what you gain from it didn't seem big enough to justify this, because at the end of the day you can accomplish a lot of the same things in git (like using the squash & merge strategy in github) without introducing any new primitives. I didn't use it long enough to really figure out if this is a purely aesthetic concern and I'm just being stubborn about the way I'm used to or if it leads to real problems in practice. I'm curious what people who really like Phabricator think about this.

sciurus 8y ago
"as in the ability to create diff objects and branch and rearrange them arbitrarily in a pretty simple way, and have multiple people working on these branches at the same time and keeping in sync with each other" I can't truly speak for them, but I suspect these are both (reworking the history of code while its in review, and multiple people working on a branch) things that the Phabricator authors would say are bad practices that your tools should discourage. The arguments that come to my mind are 1) Reworking history during review makes it near for your reviewers to understand what is changing. That should be reserved until its time to merge your code, at which point all the revisions along the way should be squashed. 2) Shared development on a branch means the scope of the change is large, and the branch is probably long-lived too. Both of these are arguably bad; I don't have time to argue why, but chapter 13 of Continuous Delivery lays it out well.
- sciurus 8y ago
  Oops, that was supposed to say "makes it near impossible", but I'm outside the window where I can edit it.
- Nullabillity 8y ago
  Mercurial's Queues extension had an interesting solution to this. Basically, WIP branches would instead be stored as patch files under version control in a separate per-PR repository. To merge you'd apply each patch as a commit and then archive/delete the queue repository. This would let you organize commits logically, while also letting reviewers query by chronological order. As a PR author it was effectively a `git rebase -i` that you could walk back and forth as you pleased, without destroying any history.
maktouch 8y ago
I like the abstraction of patches, because some of our projects still runs SVN and HG, and because we're all on Phab, we're all using the same flow. We actually don't feel any urge to move to git at all (it's probably going to happen some day anyway)
aseipp 8y ago
In practice when using Phabricator, you (or at least, I) begin to just think of branches more or less as a way of backing things up to a safe location, local incremental development, and cross-developer collaboration -- not as an actual mechanism of reviewing things that will go into the tree. Really, it's a lot like the mailing list model: you may develop incrementally in whatever way you want, probably between multiple people, but at the final stage you're going to need a clean set of delineated patches to submit for review and someone takes ownership of that part (90% of the time that's you, maybe not you if you're on a multi-person feature). These are more or less two different steps in the development cycle. This basically happens anyway in my experience at a certain point, even if using GitHub, depending on how your team develops... For example, we recently had a long-lived (~2 month) branch open that accumulated about 300 commits and diverged from our upstream master at $WORK. It implemented a large feature, between many components, and during that time had to be revised in a few ways, due to unforeseen (but minor) things. We didn't just do a direct merge; we assigned individual people subsets of the work, and they were responsible for bringing in the changes in a reliable way. This turned 300 commits into something like 30 or 40. This part of the development is basically the last step, and it took time for us to bite-size the pieces. But then the actual review step -- and submitting the patches -- is a very small part at the end. A lot of projects like LLVM follow basically the same model (they use Phabricator too but that's more a coincidence, nothing to do with the general flow): the final patch stage is a very separate part from the development stage, all things considered, and posted patches for big features rarely reflect actual development. So, I think putting yourself in this mindset helps. GitHub completely ties the notion of a branch to be both "A thing you use for development" and "The thing that the reviewer finally sees" by way of PR, but clearly this is more a design choice on their part -- rather than a fundamental property of git. Regarding abstraction, there is a thing about this, which is that you mostly interact through Arcanist to submit patches. This admittedly adds complexity and can confuse people if they think `arc` is just identical to `git`. We had to teach a lot of people to use Arcanist for Haskell.org. This can absolutely be considered a real downside. Admittedly though, Arcanist is an abstraction, and it also works for SVN and Mercurial, so if you have multiple types of repos -- that's arguably very useful! For homogeneous VCS setups, though, I can see how it only seems to add complexity. Realistically, the review stage where you interact with Arcanist is pretty small in the overall development cycle with most changes, but people are still sensitive to it. --- Honestly I don't think it's a big deal at all, and in fact I find Phabricator's model quite a bit nicer than GitHub's in some incidental ways too, because of it. One nice thing is that you don't have a trillion dead forks of popular repos, search across all patches is unified, stuff like that...

sciurus 8y ago

"as in the ability to create diff objects and branch and rearrange them arbitrarily in a pretty simple way, and have multiple people working on these branches at the same time and keeping in sync with each other" I can't truly speak for them, but I suspect these are both (reworking the history of code while its in review, and multiple people working on a branch) things that the Phabricator authors would say are bad practices that your tools should discourage. The arguments that come to my mind are 1) Reworking history during review makes it near for your reviewers to understand what is changing. That should be reserved until its time to merge your code, at which point all the revisions along the way should be squashed. 2) Shared development on a branch means the scope of the change is large, and the branch is probably long-lived too. Both of these are arguably bad; I don't have time to argue why, but chapter 13 of Continuous Delivery lays it out well.

sciurus 8y ago
Oops, that was supposed to say "makes it near impossible", but I'm outside the window where I can edit it.
Nullabillity 8y ago
Mercurial's Queues extension had an interesting solution to this. Basically, WIP branches would instead be stored as patch files under version control in a separate per-PR repository. To merge you'd apply each patch as a commit and then archive/delete the queue repository. This would let you organize commits logically, while also letting reviewers query by chronological order. As a PR author it was effectively a `git rebase -i` that you could walk back and forth as you pleased, without destroying any history.

maktouch 8y ago

I like the abstraction of patches, because some of our projects still runs SVN and HG, and because we're all on Phab, we're all using the same flow. We actually don't feel any urge to move to git at all (it's probably going to happen some day anyway)

aseipp 8y ago

In practice when using Phabricator, you (or at least, I) begin to just think of branches more or less as a way of backing things up to a safe location, local incremental development, and cross-developer collaboration -- not as an actual mechanism of reviewing things that will go into the tree. Really, it's a lot like the mailing list model: you may develop incrementally in whatever way you want, probably between multiple people, but at the final stage you're going to need a clean set of delineated patches to submit for review and someone takes ownership of that part (90% of the time that's you, maybe not you if you're on a multi-person feature). These are more or less two different steps in the development cycle. This basically happens anyway in my experience at a certain point, even if using GitHub, depending on how your team develops... For example, we recently had a long-lived (~2 month) branch open that accumulated about 300 commits and diverged from our upstream master at $WORK. It implemented a large feature, between many components, and during that time had to be revised in a few ways, due to unforeseen (but minor) things. We didn't just do a direct merge; we assigned individual people subsets of the work, and they were responsible for bringing in the changes in a reliable way. This turned 300 commits into something like 30 or 40. This part of the development is basically the last step, and it took time for us to bite-size the pieces. But then the actual review step -- and submitting the patches -- is a very small part at the end. A lot of projects like LLVM follow basically the same model (they use Phabricator too but that's more a coincidence, nothing to do with the general flow): the final patch stage is a very separate part from the development stage, all things considered, and posted patches for big features rarely reflect actual development. So, I think putting yourself in this mindset helps. GitHub completely ties the notion of a branch to be both "A thing you use for development" and "The thing that the reviewer finally sees" by way of PR, but clearly this is more a design choice on their part -- rather than a fundamental property of git. Regarding abstraction, there is a thing about this, which is that you mostly interact through Arcanist to submit patches. This admittedly adds complexity and can confuse people if they think `arc` is just identical to `git`. We had to teach a lot of people to use Arcanist for Haskell.org. This can absolutely be considered a real downside. Admittedly though, Arcanist is an abstraction, and it also works for SVN and Mercurial, so if you have multiple types of repos -- that's arguably very useful! For homogeneous VCS setups, though, I can see how it only seems to add complexity. Realistically, the review stage where you interact with Arcanist is pretty small in the overall development cycle with most changes, but people are still sensitive to it. --- Honestly I don't think it's a big deal at all, and in fact I find Phabricator's model quite a bit nicer than GitHub's in some incidental ways too, because of it. One nice thing is that you don't have a trillion dead forks of popular repos, search across all patches is unified, stuff like that...