Heh. Your list of UI complaints is very similar to mine. Some comments:
On 01/25/2016 04:26 AM, Honza Bambas wrote:
Writing both as a patch author and a reviewer as well.
- as a patch author I want a full control on when the patch actually
lands (dependencies, any other timing reasons, that only the author
knows the best), "Don't land yet" comment somewhere will easily be
overlooked
- as a reviewer I don't want to bare the decision to land or not to
land, at all
- I want to preserve the mechanism "r+ with comments", which means to
let the patch be landed by the author after updated (means reviewer
doesn't need to look over it again)
- as an author I want to write comments about the patch (about the
structure, what it does, why) to make the review simpler for the
reviewer ; commit message description may not be the best place, right?
Yes, is there a place for this? I feel this is a really important bit of
functionality that would fit naturally into the mozreview world, but I
don't see how to do it. Situation: I put up a set of commits for review,
and I want to give per-commit notes to the reviewer that they'll see
before reviewing. Previously, I would put them in as comments on the bug
and either pray that the reviewer happened to see them, or ping the
reviewer on IRC and tell them to read them. In MozReview, I don't see a
place to put such things at all?
- will it be possible to still be using hg patch queues?
A valid question, though I'd not that the mq-less workflow is actually
pretty good these days. mq is still easier/nicer for some things, but
doing without mq is nicer in other ways, and the future leads away from mq.
On the other hand, there still isn't a great way to figure out the
mq-less workflow. I remember a pretty good writeup from someone, and I
intend to write up my own. But there isn't anything that's easily
discoverable.
- I (and others too) am not fun of MozReview UI. As a reviewer I
found it very hard to orient in it:
- what is the difference between [Reviews] and [Diff] tab? what is
exactly it's content
I just attempted to use MozReview again recently, and from what I can
tell: there are kind of two main modes, I'll call them "overall" and
"per-commit". There's is an overall pane that sticks to the top,
incidentally making it difficult to figure out whether you're in the
overall or per-commit mode. Clicking on "Review Summary" goes to the
overall review mode -- as in, the metadata about the whole review. Below
the fixed pane is the history of metadata changes (so "Review summary"
means "Overall review metadata view", or something.) Clicking on
"Squashed diff" is also for the overall review mode, but now it shows
the squashed diff. In the overall mode, there's some text that claims
that you can leave comments on the squashed diff, but advises against
it. There's also no obvious way to leave such comments, but I just
figured out that it must mean that you can select regions of code and
it'll pop up a comment box. AFAICT, there's no way to leave general
comments.
Also, on a large (overall) change, the UI goes off and spends so much
time loading in all of the changed chunks that it doesn't have time to
be responsive to button clicks and things, so it's really hard to tell
what is working and what isn't. But my browser is suffering for other
reasons these days (yeah, Nightly really sucks for me right now), so
that may be local.
You get into "per-commit" mode by clicking on the description of a
review. That will try to trick you by leaving the overall summary in
place at the top, but if you are careful you'll notice that the portion
below it is now specific to one commit.
Oh! In writing this, I finally figured out the data model. You have two
orthogonal dimensions: "review vs diff" and "overall vs per-commit".
"Review Summary" means "overall x review". "Squashed diff" means
"overall x diff". The summary table at the top lists links that will get
you to per-commit views for each commit. The ones in the "Diff" column
are "per-commit x diff". The textual descriptions in the "Reviews"
column are "per-commit x review". The two
tabs-that-still-aren't-true-tabs in the top right control just the
"review vs diff" dimension for either the overall or the per-commit views.
\o/ It's starting to finally make sense to me! (I think?)
Then there are the tabs-that-aren't-even-close-to-being-tabs, which have
various context-specific actions on them.
> - where exactly to click to start a reivew of a patch I want to
review now? Is in the "Commits" table? And is it under "Diff" or
"Reviews"?
To leave a comment, it looks like you have to go to the Commits table,
under the "Diff" column, and then select line regions.
It took me a while to figure out the latter part, btw. I selected a
region of code to comment on it. No luck. I double-clicked a line of
code. No luck. I eventually figured out that it wants you to only use
the line numbers -- you can double-click on, or drag out a region, and
it'll open up a comment box. This has a discoverability problem, and
personally I think it should allow you to select the code text directly
(probably at a line granularity.)
And let you do whole-review, whole-commit, and whole-file comments.
- how can I mark hunks/files are already reviewed (something I
like on Splinter)?
There's a large "not reviewed"/"reviewed" toggle in the top right above
each chunk.
- how can I see only a single file diff and easily navigate
between files? (as in Splinter)
Use blue painter's tape to mask out the rest of the screen showing other
files. Don't leave it on for too long, or it'll leave gunk on your
monitor. For navigation, there's a list of changed files at the top
(below the fixed summary pane) that jumps to per-file anchors. Are you
saying you want tabs or something for this (like splinter uses)? I'd
certainly like something less sluggish, but maybe that's just my browser
again.
- few weeks ago I didn't even know how to give an r+!! it's
hidden under the [Finish review...] *tab*?
Yeah, MozReview has rampant tab abuse.
- simply said: the UI is everything but self-explanatory and
highly unfriendly, until that is fixed I'm not much willing to use
MozReview mainly as a reviewer
It makes more sense once you figure out the data model, though it can
feel at times like it's actively trying to prevent you from grokking
that model. I haven't used it too many times yet, and I'd like to file
bugs for everything that bothers me, but every time I try I end up with
a page long listing and I feel like some of the issues are too systemic
to really be addressed properly with individual bugs. (And yes, I do
post my page-long diatribes to the mailing list, and a number of things
are better now but I still don't feel like it has come together yet.)
In brief, my main issues are:
- tab abuse
- the data model is nonobvious and difficult to discover
- needs to do better at facilitating communication between the author
and the reviewers
I still like the basic model of what it's trying to do. It's really nice
to be able to push a half-dozen commit series and then re-push fixed and
rebased changes without uploading one patch at a time. And basing it on
real commits (pullable, shareable, etc.) is handy. It's still not to the
point where I'd actually want to use it over bugzilla+splinter, though.
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
