The future of commit access policy for core Firefox

smaug smaug at welho.com
Sun Mar 12 20:53:04 UTC 2017


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 <
>>> governance at 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)


> 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)




More information about the firefox-dev mailing list