<div dir="ltr">IMO the right place for this checkbox is in the review request, which puts the<div>control in the right place: the patch author.<br><div><br></div><div>-Ekr</div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 21, 2016 at 7:00 PM, Dave Townsend <span dir="ltr"><<a href="mailto:dtownsend@mozilla.com" target="_blank">dtownsend@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Should we just add a "and land it" checkbox to the review page, maybe<br>
disabled if there are still open issues?<br>
<div><div class="h5"><br>
On Thu, Jan 21, 2016 at 6:35 PM, Gregory Szorc <<a href="mailto:gps@mozilla.com">gps@mozilla.com</a>> wrote:<br>
> If you have level 3 source code access (can push to central, inbound,<br>
> fx-team) and have pushed to MozReview via SSH, as of a few weeks ago you can<br>
> now land commits from the "Automation" drop down menu on MozReview. (Before<br>
> only the review request author could trigger autoland.)<br>
><br>
> This means that anyone [with permissions] can land commits with a few mouse<br>
> clicks! It will even rewrite commit messages with "r=" annotations with the<br>
> review state in MozReview. So if someone does a drive-by review, you don't<br>
> have to update the commit message to reflect that reviewer. Neato!<br>
><br>
> I've gotten into the habit of just landing things if I r+ them and I think<br>
> they are ready to land. This has startled a few people because it is a major<br>
> role reversal of how we've done things for years. (Typically we require the<br>
> patch submitter to do the landing.) But I think reviewer-initiated landing<br>
> is a better approach: code review is a gate keeping function so code<br>
> reviewers should control what goes through the gate (as opposed to patch<br>
> authors [with push access] letting themselves through or sheriffs providing<br>
> a support role for people without push access). If nothing else, having the<br>
> reviewer land things saves time: the ready-to-land commit isn't exposed to<br>
> bit rot and automation results are available sooner.<br>
><br>
> One downside to autoland is that the rebase will happen remotely and your<br>
> local commits may linger forever. But both Mercurial and Git are smart<br>
> enough to delete the commits when they turn into no-ops on rebase. We also<br>
> have bug 1237778 open for autoland to produce obsolscence markers so<br>
> Mercurial will hide the original changesets when you pull down the rebased<br>
> versions. There is also potential for some Mercurial or Git command magic to<br>
> reconcile the state of MozReview with your local repo and delete local<br>
> commits that have been landed. This is a bit annoying. But after having it<br>
> happen to me a few times, I think this is a minor annoyance compared to the<br>
> overhead of pulling, rebasing, rewriting commit messages, and pushing<br>
> locally, possibly hours or days after review was granted.<br>
><br>
> I encourage my fellow reviewers to join me and "just autoland it" when<br>
> granting review on MozReview.<br>
><br>
> gps<br>
><br>
</div></div>> _______________________________________________<br>
> firefox-dev mailing list<br>
> <a href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a><br>
> <a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listinfo/firefox-dev</a><br>
><br>
_______________________________________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listinfo/firefox-dev</a><br>
</blockquote></div><br></div></div></div></div>