Re: The future of commit access policy for core Firefox

Nicholas Nethercote <[email protected]> Sat, 11 Mar 2017 12:02:30 +1100
Newsgroups gmane.comp.mozilla.devel.seamonkey
Message-ID <CAEXw-AD84PkZv7D9PxDUNKW8-oo+8gVEGYwoUqV98qfaZY7CDw@mail.gmail.com>
On Sat, Mar 11, 2017 at 4:49 AM, Mike Connor <[email protected]> wrote:

>
> To expand a bit on that last point: if we can trust core/known developers
> to competently make necessary changes prior to checkin, we should also be
> able to expect them to fix most/all of those issues *before* they submit
> those patches for review.  My experience as both author and reviewer has
> generally been that doing code review on something 90% finished takes far
> more time than something that's been refined thoroughly prior to
> submission.  I'd love to see us tighten up the expectations on patch
> authors to only request review when they believe something is fully ready
> to land.  (If you want early/high-level feedback, there's a flag for that.)
>

I frequently makes minor changes to patches after review that haven't been
suggested by reviewers. Often after sleeping on it. These are probably more
often in comments than code. Maybe I'm unusual in this respect.

I'm surprised that Servo and Rust haven't come up yet, because they do
already have a model much like the one suggested -- landing approval must
come from an authorized reviewer.

In my experience it's not a disaster but it is a hassle at times and
motivates against suggesting (as a reviewer) or making (as an author) minor
changes -- it's easy to be lazy and say "close enough". And in the case
where minor changes are requested, the final approval often is a complete
rubber stamp -- I've had Servo patches authorized by people uninvolved with
the PR simply because they were in my timezone and so online at the time.

Nick