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 _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
