Re: The future of commit access policy for core Firefox

Nathan Froyd <[email protected]> Mon, 13 Mar 2017 14:35:02 -0400
Newsgroups gmane.comp.mozilla.devel.seamonkey
Message-ID <CAGepuV+0poeCms7NrxCRK-sC6c-DLQCrvVzJmDiXLNGR2TO0ZA@mail.gmail.com>
On Mon, Mar 13, 2017 at 2:12 PM, Mike Connor <[email protected]> wrote:
> While there's some fuzzy lines between modules (and the pan-tree rewrite
> case needs a bit more discussion), I think we can quickly to get to a point
> where module owners and peers are defined in-tree, in a machine-readable
> format that we could use to a) validate that the reviewer(s) of a patch are
> the appropriate people and b) suggest potential reviewers.

I think you're going to want to handle delegation of reviews, e.g.

1) I own/peer a module and I would like somebody to review my patches
so I can get a sense of how thoroughly they review code and/or to help
provide feedback on their review skills.

2) Similar to the above, maybe I get tagged for review, but I would
like somebody else who doesn't own/peer the module to review the code
because I judge them capable enough.  This and the above case are
mostly about growing new reviewers.

2a) Perhaps I would just like a second set of eyes on the patch (e.g.
threading issues) and am comfortable handing off review to somebody
who is more expert in such matters and whose judgement I trust to
handle any other issues capably.

3) Sometimes I get asked about reviewing particular things and I ask
the person who I know wrote the original code to review instead
because they are more familiar than I am.  Maybe this can be addressed
with ever-finer lists of code owners.

4) Would just like to reiterate that the rewriting code case needs
some thought, as somebody who has done such rewrites (and appreciated
getting a single review) and as somebody who has reviewed code for
such rewrites.

If we require reviewers to appear in a trusted-list in-tree, then
actually growing new reviewers is going to be a real pain.

> * patch authors who are *also* owners/peers or otherwise marked as trusted
> for the code in question (membership in SR group, or explicitly trusted by
> the module owner) are permitted to carry over a review.

This idea could also be expanded to handle the delegation cases above,
of course.

-Nathan