|
|
|
|
|
by drewg123
3770 days ago
|
|
I recently started using Phabricator fairly heavily for a review of a large patchset, and I am less than impressed. It could be that the project where I'm using Phabricator didn't set up their review system properly, it could be related to them running svn, etc. And it could certainly be due to my having used Gerrit for 2+ years and Phabricator for a few months. That said.. The problem I have with Phabricator can be summed up in saying that it is a review system which is not tightly integrated with the repo, and that approach has inherent problems. If you actually want to apply the patchset under review in order to run it (or even look at it with other tools), arcanist does not seem to be helpful in finding the correct revision of the tree. You have to communicate with the author via side channels to determine what version of the tree they were running when the patch was created. When arc patch blows up, it is not clear how to recover (picture a git merge with no obvious way to resolve conflicts). I finally got a pointer to the author's git repo and cloned it.. I find it a bit disingenuous that the essay first compares Phabricator to Github pull based workflows, and to gerrit. They go on to criticize pull based workflows, but fail to mention that Gerrit is not a pull based workflow, and gives the same advantages of Phabricator (squashed, linear history). Gerrit has the advantage of being tightly integrated with a repo, and being able to easily fetch the author's work so you can look at it in context, test it, etc. |
|
I think the local patch problem you are experiencing is due to the way the author submitted their diff. Normally if you want to locally try code you run "arc patch D123" and it will create a branch off that revision and apply the patch for you. If you look at this review [1] you see it has a link to the repository "Repository rP Phabricator", and in the "Revision Update History" and "Local Commits" sections it has direct links to the base and parent commits. If the submitter based their diff off non-published commits this won't work.
You can also reduce the need to directly pull down code by integrating a CI tool, code coverage information, and screenshots into your review process. All of those Phabricator supports.
[0] https://secure.phabricator.com/book/phabflavor/article/recom...
[1] https://secure.phabricator.com/D15358