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 <[email protected]> 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 > [email protected] > https://mail.mozilla.org/listinfo/firefox-dev > _______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
