On Sunday, March 12, 2017 at 1:23:45 AM UTC+11, smaug wrote: > On 03/11/2017 08:23 AM, Nicholas Nethercote wrote: > > On Sat, Mar 11, 2017 at 2:23 PM, smaug via governance < > > gov...@lists.mozilla.org> wrote: > >> > >> I'd be ok to do a quick r+ if interdiff was working well. > > > > Depending on the relative timezones of the reviewer and reviewee, that > > could delay landing by 24 hours or even a whole weekend. > > > The final r+, if it is just cosmetic changes wouldn't need to be done by the > same reviewer. > > Perhaps we shouldn't even call the last step a review. It would be > "ok-to-land". > r+ without asking any changes would implicitly contain that "ok-to-land". > (if rebasing causes some changes, that would then need explicit "ok-to-land") > > > > > > In general there seems to be a large amount of support in this thread for > > continuing to allow the r+-with-minor-fixes option. > > Yeah. I don't object that, but I also think that having final approval to > land the patch might not really be that bad > (assuming the tools are working well enough). > > > > > Nick
If we really want to enforce a final review to prevent unwanted code to land, Mozilla (as a whole) will incur some costs: Reviewers spending time re-reviewing; delays to land (worse across tz, and which could result in the need to rebase again, and therefore another round of ok-to-land); and mounting anger at all these papercuts. So if Mozilla is really committed to this solution, and is ready to bear the associated financial costs, I would suggest we recruit people who would do just these tasks! Could be existing sheriffs, and/or volunteering peers, and/or new staff with distinct job descriptions. Hopefully they'd rubber-stamp 99% of last-minute changes, and only the more complex changes would be referred back to the author and reviewer. As a bonus they could also handle trivial autoland merge issues. In the end it should cost a similar amount of money, but would lessen the other costs (delays and frustration). And while I've got the mic, a small suggestion for mozreview to help with some of this: We need a way for the author to add a comment explaining what was done between pushes (rebase, nit-fixing, other notes to reviewer, etc.); this comment would only appear in Bugzilla and mozreview, not in the actual patch commit description. (Could be a paragraph starting with "notes-to-reviewer:", to be removed before autolanding.) - Gerald _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
