Hi, I'd like to conclude my Phabricator investigation now. Indeed I have spent a lot less time on it than on GitHub, however my reason for this earlier finish is not bias or exhaustion. I believe to have found some pretty critical functionality gaps in Phabricator, which make it "secondary" how the emails it sends look and such.
(The URLs we used for investigation are: https://code.bluestop.org/D1 https://code.bluestop.org/D2 https://code.bluestop.org/D3 https://code.bluestop.org/D4 ) Namely: Phabricator doesn't integrate natively with git topic branches, or with "git" itself, for that matter. Submitting a series of commits (a topic branch) for review seems to require a tool called "arcanist" (or "arc" for short), which from my perspective is a very unwelcome requirement. (Earlier I expressed my hope that it would be an optional tool.) Rebecca pointed me to the following article: https://smacleod.ca/posts/commit-series-with-phabricator/ and while the method described might technically work, I think it's a large step back in usability if we need to dance around the system with an extra tool (which isn't even packaged by several Linux distributions currently), just for submitting a topic branch for review. In addition, the way a commit series is represented in a Phabricator (for the purposes of comprehensive review), AIUI, is a chain of inter-dependent, but *separate* review requests. While the article above describes a method for interleaving a local "git rebase -i" procedure with re-stacking the patches / review requests on the Phabricator server, it strongly feels like a workaround for a system for which a *series* of patches is only an afterthought, not the core idea. (In git, branches are the core idea -- creating them is very cheap, and merging them is actually possible.) In other words, even though setting up the local commit messages as required by Phabricator, and running "arc diff" in a loop basically, might obviate a manual struggle on the WebUI, for stacking multiple review requests (one per patch) into a series, I don't see how it could work in any sensible way with the size of the patch sets that we regularly post. 10-20 patches in an edk2 series are totally normal, and 50+ patches in a series are not unheard of. Yes, 50+ patches in a series are not frequent, but long series are *exactly* when you need your tooling to support you the most. I can't see myself integrating an external tool with "git rebase" (esp. one that might call out over the network, such as "arc diff") when I'm trying to move code hunks between patches in a 20+ part series. When polishing a large series, it's not uncommon to rebase the whole thing multiple tens of times. Yet further, according to Rebecca's explanation in <https://code.bluestop.org/D1>, fetching a patch under review for local testing again requires "arc". This is not native to git, and I think it causes more problems than it solves. Also, what about fetching a series of patches, i.e. a series of dependent review requests... The local git command line works *already* (once we're given a remote URL and a branch or tag), so let's not mess with that please. To be honest, I'm stumped how Mozilla could adopt (according to the article linked at the top) "Phabricator as the primary code review system for Firefox". Out of this exercise, I'd like to suggest a hard requirement for all future candidates: native integration with "git" is a must, and the system must not require additional command line tools for basic git operations (such as push, fetch, rebase), and basic participation in development. Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
