Regarding comment and flag mirroring, we've discussed this before:
https://groups.google.com/d/msg/mozilla.dev.platform/Y8kInYxo8UU/e3Pi-_FpBgAJ
https://groups.google.com/d/msg/mozilla.dev.platform/Y8kInYxo8UU/tsF7UfxvBgAJ
Given that Phabricator is still new, I don't see any reason to reopen that discussion at this point, aside from noting that we have work in progress to include Phabricator requests in BMO's My Dashboard and notifications indicator (
https://bugzilla.mozilla.org/show_bug.cgi?id=1440828).
As for interdiffs, feel free to file a bug with any problems you see. We have a good relationship with upstream and can pass those on. Similarly with method names (which has been mentioned before but I can't find where at the moment).
There is official documentation at
https://secure.phabricator.com/book/phabricator/ which is linked from our Mozilla-specific docs (
http://moz-conduit.readthedocs.io/en/latest/phabricator-user.html) which in turn is linked in the left-hand menu in Phabricator. We can expand our own docs as needed if there are areas that are particularly confusing due to, say, expectations carried over from our other code-review tools.
Mark
On Thursday, 29 March 2018 13:45:28 UTC-4, smaug wrote:
> 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