IMO the right place for this checkbox is in the review request, which puts the control in the right place: the patch author.
-Ekr On Thu, Jan 21, 2016 at 7:00 PM, Dave Townsend <dtowns...@mozilla.com> wrote: > Should we just add a "and land it" checkbox to the review page, maybe > disabled if there are still open issues? > > On Thu, Jan 21, 2016 at 6:35 PM, Gregory Szorc <g...@mozilla.com> 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 > > > > _______________________________________________ > > firefox-dev mailing list > > firefox-...@mozilla.org > > https://mail.mozilla.org/listinfo/firefox-dev > > > _______________________________________________ > firefox-dev mailing list > firefox-...@mozilla.org > https://mail.mozilla.org/listinfo/firefox-dev > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
