Hi all,
just some random notes about Phabricator.
I've been reviewing now a bunch of patches in Phabricator and the initial
feeling from
reviewer's point of view is that it is ok. Not great, but ok.
MozReview's interdiff, when it works, is easier to use or at least to discover
than Phabricator's.
MozReview does also show the method name in which the code lives. This latter
thing is actually a big
+ to MozReview. (Same works in Bugzilla too when reviewing raw -P patches at
least. No idea about splinter, since I never use it).
If Pabricator has the feature, I haven't found it yet - its UI is a tad
confusing occasionally.
What is currently rather broken is the flag synchronization between bugzilla and Phabricator. One doesn't see in Bugzilla whether review has been
asked or whether review has been denied (r-). I believe people asking reviews from me set the r? flag manually in bugzilla.
Having an easy way to see that r- has been given to a patch is very valuable to
the reviewer when looking at the bug again.
Oddly enough, approving the patch in Phabricator does set r+ in Bugzilla.
Not mirroring comments from Phabricator to Bugzilla has already made following
reasoning for certain code changes harder.
It is so easy to include non-code level information in review comments. So far
I haven't had a case where I would have needed to look at the blame and
try to find relevant information both in bugzilla and in phabricator, but that
will obviously happen if we don't do the mirroring and it will slow
down reviewing in the future (since looking at the history/reasoning for the
code changes is a big part of code reviewing).
I'm sure I haven't found all the tricks Phabricator has to help reviewing. Is
there some reviewing-in-Phabricator-for-dummies doc?
I haven't asked reviews in Phabricator (nor in MozReview), so can't comment on
how they compare from patch author's point of view.
br,
-Olli
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
