On 2017-03-12 4:53 PM, smaug wrote: > On 03/12/2017 10:40 PM, Ehsan Akhgari wrote: >> On 2017-03-11 9:23 AM, smaug via governance wrote: >>> On 03/11/2017 08:23 AM, Nicholas Nethercote wrote: >>>> On Sat, Mar 11, 2017 at 2:23 PM, smaug via governance < >>>> governa...@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. >> >> OK, but what is the exact time zone efficient way to ensure that no huge >> amount of delay is added for the turn around of this final review round? >> >> Let's be realistic. In practice, adding one extra person in the process >> of landing of the patches that currently land with r+-with-nits *will* >> slow them down. And we should expect that it will slow them down by >> factors such as time zones, people missing bugmail, etc, > > Why we need to expect that? In my mind the process would be closer to > "I want to land this patch, asking ok-to-land on irc." And then someone > who has the rights to say ok to that would > mark the patch after seeing that the interdiff doesn't add anything > inherently bad. > (well working interdiff and tooling in general is rather critical)
Depending on the timezone, finding people on IRC may be challenging. For example, many of our colleagues in Taipei may find this kind of IRC reviews difficult if the people who need to look at the change are on the other side of the planet. >> all of the >> reasons why currently one review can end up taking a week, > I see this being _very_ different to normal reviews. > > I do understand that people don't want extra process. Overhead is always > overhead. > But the overhead here might not be as bad as feared. > >> it may end up >> being that final round of review after this proposed change. >> >> And I still don't understand what the proposal means with rebases in >> practice. What if, after automation tries to land your change after you >> got your final r+ the final rebase fails and you need to do a manual >> rebase? Do you need to get another r+? What happens if you're touching >> one of our giant popular headers and someone else manages to land a >> conflict while your reviewer gets back to you? >> >>> 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") >> >> Are you suggesting a change to the nature of the review process for the >> last step with the rename? > > The last step here would be quite different to normal reviews. It is > just a rather quick glance-over that no > obviously evil code is about to land. (as I said, well working interdiff > is absolutely critical to make this working) OK, this distinction is key. To me this seems adding process for the sake of adding process. Is this really going to be all that much better at preventing vulnerabilities from sneaking in than what we have today? _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
