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?
- will it be possible to still be using hg patch queues?
- 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
- 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"?
- how can I mark hunks/files are already reviewed (something I like
on Splinter)?
- how can I see only a single file diff and easily navigate between
files? (as in Splinter)
- few weeks ago I didn't even know how to give an r+!! it's hidden
under the [Finish review...] *tab*?
- 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
-hb-
On 1/22/2016 3:35, Gregory Szorc wrote:
If you have level 3 source code access (can push to central, inbound,
fx-team) and have pushed to MozReview via SSH, as of a few weeks ago you
can now land commits from the "Automation" drop down menu on MozReview.
(Before only the review request author could trigger autoland.)
This means that anyone [with permissions] can land commits with a few mouse
clicks! It will even rewrite commit messages with "r=" annotations with the
review state in MozReview. So if someone does a drive-by review, you don't
have to update the commit message to reflect that reviewer. Neato!
I've gotten into the habit of just landing things if I r+ them and I think
they are ready to land. This has startled a few people because it is a
major role reversal of how we've done things for years. (Typically we
require the patch submitter to do the landing.) But I think
reviewer-initiated landing is a better approach: code review is a gate
keeping function so code reviewers should control what goes through the
gate (as opposed to patch authors [with push access] letting themselves
through or sheriffs providing a support role for people without push
access). If nothing else, having the reviewer land things saves time: the
ready-to-land commit isn't exposed to bit rot and automation results are
available sooner.
One downside to autoland is that the rebase will happen remotely and your
local commits may linger forever. But both Mercurial and Git are smart
enough to delete the commits when they turn into no-ops on rebase. We also
have bug 1237778 open for autoland to produce obsolscence markers so
Mercurial will hide the original changesets when you pull down the rebased
versions. There is also potential for some Mercurial or Git command magic
to reconcile the state of MozReview with your local repo and delete local
commits that have been landed. This is a bit annoying. But after having it
happen to me a few times, I think this is a minor annoyance compared to the
overhead of pulling, rebasing, rewriting commit messages, and pushing
locally, possibly hours or days after review was granted.
I encourage my fellow reviewers to join me and "just autoland it" when
granting review on MozReview.
gps
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
