Re: Proposing changes to granting CVS access, Round 2
Mitchell Baker
I'm cross-posting this to mozilla.governance, and setting the follow-up
there. Now that we've started using that newsgroup it's the right place
for this set of discussions.
First I'd like to summarize the changes from the current policy and make
sure I'm understanding. So lots of questions follow, and (I think) no
expression of opinion.
(As a side note, I filed a bug to clean up the super-reviewer list and
look at how super-review is used today: 366103.)
Proposed Changes
1. Required Approvals
-- In the old world, one needed a voucher and 2 or 3 super-reviewers
(2 if the voucher was a super-reviewer, 3 if s/he wasn’t).
-- In the proposed new world, one needs three module owners, or three
peers and/or three super-reviewers
-- In other words, we’re now saying that module owners and peers can
take the place of super-reviewers. Mike, can you verify this is the intent?
2. Role of Vouchers
-- In the old world, the “voucher” was responsible for the “vouchees”
behaviour and correcting early mistakes. The super-reviewers didn’t
take this responsibility on officially, their role was to vett
understanding across the codebase.
-- In the proposed new world we have 3 vouchers. Mike, are you
proposing that all 3 vouchers take on the responsibility for the effects
of the vouchee’s check-ins?
3. Breath of Vouchers
-- In the old world the use of super-reviewers required an
understanding of interactions between modules and what we called
“integration”; a short-hand for some set of coding practices required to
work well with other modules.
-- In the proposed new world ll 3 vouchers could be from the same module
4. Criteria for CVS Access
-- In the old world the criteria was understanding of Mozilla
architecture and effect of one’s changes on others. The current
enumerated items probably need updating.
-- Are you proposing something different? Your suggestion sounds more
like “can fix bustage” than a focus on the breath of the code. This may
be appropriate, no value judgement intended (at least not yet!).
-- In the old world the focus was on code quality
-- In the proposed new world the focus seems more on workstyle. Is
this intended?
-- In the old world we required enough patches for people to be able
to vouch for code quality
-- In the proposed new world it seems to me the focus is on enough
time to have a feel for the person. Have I got this right?
5. Employement of vouchers
Same as the old world – one must not have the same employer.
mitchell
Mike Connor wrote:
> To see the prior history on this, check out
> http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/f1636813ca66e35c/888f829c6f368205?
>
>
> The key to me is that as we get more and more areas of the code working
> under strong module ownership, and not having superreview, it becomes
> harder for contributors to obtain full CVS access.
>
> In essence, I want to propose the amended points below
>
> To be granted access to mozilla.org CVS, a contributor must meet the
> following guidelines
>
> * Recognized in and involved with the community (outside of your own
> co-workers)
> * Understands and follows our process (being on hook, awareness of tree
> rules/state)
> * Knows enough about the codebase to work without a net (i.e. can fix
> bustage)
> * Is responsible about addressing review comments
> * Has enough of a track record to indicate the above (at least a couple
> of months of active hacking)
>
> Based on those criteria, I would propose we change the vouching
> process to be as follows:
>
> * All CVS account requests require a total of three vouchers (except for
> NSS/NSPR/Spidermonkey)
> * Vouchers must be module owners/peers for a module the individual has
> patched or a superreviewer
> * At least one voucher should have a different employer than the
> requestee (goes to community involvement more than anything else)
>
> Thoughts?
>
> -- Mike
Mitchell Baker
Mike Connor
On 5-Jan-07, at 8:34 PM, Mitchell Baker wrote:
> Proposed Changes
>
> 1. Required Approvals
>
> -- In the old world, one needed a voucher and 2 or 3 super-
> reviewers (2 if the voucher was a super-reviewer, 3 if s/he wasn’t).
> -- In the proposed new world, one needs three module owners, or
> three peers and/or three super-reviewers
> -- In other words, we’re now saying that module owners and peers
> can take the place of super-reviewers. Mike, can you verify this
> is the intent?
That is the intent. Super-review is being used for smaller pieces of
the code than it has in a long time. In terms of the front end,
there is no SR required in browser/toolkit/PSM, which covers all of
the UI I can think of, offhand. We have had a longstanding policy
about only needing one voucher for non-SR modules, but I don't think
that's quite enough, or well-balanced. For modules still requiring
SR, many of the owners/peers are also SRs.
> 2. Role of Vouchers
>
> -- In the old world, the “voucher” was responsible for the
> “vouchees” behaviour and correcting early mistakes. The super-
> reviewers didn’t take this responsibility on officially, their role
> was to vett understanding across the codebase.
> -- In the proposed new world we have 3 vouchers. Mike, are you
> proposing that all 3 vouchers take on the responsibility for the
> effects of the vouchee’s check-ins?
That's the idea, yes. It actually effectively raises the bar of
trust, as you have to develop a good relationship with more people
> 3. Breath of Vouchers
>
> -- In the old world the use of super-reviewers required an
> understanding of interactions between modules and what we called
> “integration”; a short-hand for some set of coding practices
> required to work well with other modules.
> -- In the proposed new world ll 3 vouchers could be from the same
> module
Correct. For non-SR modules this is actually a higher bar than
currently exists.
> 4. Criteria for CVS Access
>
> -- In the old world the criteria was understanding of Mozilla
> architecture and effect of one’s changes on others. The current
> enumerated items probably need updating.
> -- Are you proposing something different? Your suggestion sounds
> more like “can fix bustage” than a focus on the breath of the
> code. This may be appropriate, no value judgement intended (at
> least not yet!).
I don't think a broad understanding of the entire codebase is
required for the type of work many people are doing. One needs to
know enough to understand impact of their changes, but that amount of
knowledge will vary broadly depending on the area of code someone is
working within.
> -- In the old world the focus was on code quality
> -- In the proposed new world the focus seems more on workstyle.
> Is this intended?
Not at all, I would expect that the standard for vouching remains as
high as it currently is. I should be much more explicit there.
> -- In the old world we required enough patches for people to be
> able to vouch for code quality
> -- In the proposed new world it seems to me the focus is on enough
> time to have a feel for the person. Have I got this right?
I probably put too much weight on the workstyle piece in general, but
I think both are equally important.
> _______________________________________________
> governance mailing list
> gover...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/governance
Mike Shaver
> Sorry for the delay here, this ball fell on the floor.
>
> On 5-Jan-07, at 8:34 PM, Mitchell Baker wrote:
>
> > 2. Role of Vouchers
> >
> > -- In the old world, the "voucher" was responsible for the
> > "vouchees" behaviour and correcting early mistakes. The super-
> > reviewers didn't take this responsibility on officially, their role
> > was to vett understanding across the codebase.
> > -- In the proposed new world we have 3 vouchers. Mike, are you
> > proposing that all 3 vouchers take on the responsibility for the
> > effects of the vouchee's check-ins?
>
> That's the idea, yes. It actually effectively raises the bar of
> trust, as you have to develop a good relationship with more people
I think I like where this points us, but I'd like to be clear: for
what effects are the vouchers taking responsibility? Other than the
reputation effects of repeated tree bustage or shaky code trickling
through the reviewer community, there aren't many other effects of
bogus checkins that I can think of right now. I guess we might
(maybe?) call people at home in the case of serious software
shenanigans, but I think these days we'd mostly just back people out.
Can you give some examples of things that currently fall only on the
shoulders of the committer, and which would be borne also by the
vouchers in the new world?
Mike
Mike Connor
> Can you give some examples of things that currently fall only on the
> shoulders of the committer, and which would be borne also by the
> vouchers in the new world?
I don't know if I'd express voucher's responsibilities in terms of
equal to the committer, but more of a mentorship type role. Lemme
throw out some explicit ideas, none of which I'm committed to, but
these are probably all somewhat sane:
* New committers are on probation for three months, during that time
vouchers should keep tabs on their checkins and be aware of issues
(bustage and not present, persistent lack of awareness of closed
trees, regressions not acted upon). Vouching should not be a one-
off, its a committment to work with that person to get them fully up
to speed.
* If someone's access turns out to be problematic, any of the
vouchers should be able to have the individual's access disabled
until the individual demonstrates an understanding of what needs to
change (maybe directly via despot w/hacks?)
* If there are problems, and the vouchers do not take action to
address it themselves, they will not be allowed to vouch for others
in the future.
Added thought:
* New committers cannot vouch for others until they've completed the
three months.
-- Mike
Mitchell Baker
access -- they are below. These seem like a reasonable set of
responsibilities to me, and I'd be content to work with these.
Does anyone have misgivings? And if so, a specific, alternative proposal?
If not, then I'm inclined to work with this as I prepare an official
update of the policy.
Mitchell
Mike Connor wrote:
> On 16-Jan-07, at 10:43 AM, Mike Shaver wrote:
>
>> Can you give some examples of things that currently fall only on the
>> shoulders of the committer, and which would be borne also by the
>> vouchers in the new world?
>
> I don't know if I'd express voucher's responsibilities in terms of equal
> to the committer, but more of a mentorship type role. Lemme throw out
> some explicit ideas, none of which I'm committed to, but these are
> probably all somewhat sane:
>
> * New committers are on probation for three months, during that time
> vouchers should keep tabs on their checkins and be aware of issues
> (bustage and not present, persistent lack of awareness of closed trees,
> regressions not acted upon). Vouching should not be a one-off, its a
Mitchell Baker
access -- they are below. These seem like a reasonable set of
responsibilities to me, and I'd be content to work with these.
Does anyone have misgivings? And if so, a specific, alternative proposal?
If not, then I'm inclined to work with this as I prepare an official
update of the policy.
Mitchell
Mike Connor wrote:
> On 16-Jan-07, at 10:43 AM, Mike Shaver wrote:
>
>> Can you give some examples of things that currently fall only on the
>> shoulders of the committer, and which would be borne also by the
>> vouchers in the new world?
>
> I don't know if I'd express voucher's responsibilities in terms of equal
> to the committer, but more of a mentorship type role. Lemme throw out
> some explicit ideas, none of which I'm committed to, but these are
> probably all somewhat sane:
>
> * New committers are on probation for three months, during that time
> vouchers should keep tabs on their checkins and be aware of issues
> (bustage and not present, persistent lack of awareness of closed trees,
> regressions not acted upon). Vouching should not be a one-off, its a
Mitchell Baker
close I think -- many thanks to mconnor for getting this going.
Mike Connor wrote:
>
>> 3. Breath of Vouchers
>>
>> -- In the old world the use of super-reviewers required an
>> understanding of interactions between modules and what we called
>> “integration”; a short-hand for some set of coding practices required
>> to work well with other modules.
>> -- In the proposed new world ll 3 vouchers could be from the same module
>
> Correct. For non-SR modules this is actually a higher bar than
> currently exists.
I understand the motivation here. I think this results in people being
able to check code into open modules (say Gecko) without any one of the
Gecko folks or super-reviewers having been involved in granting CVS
access. This may be OK -- all code needs review from module owners or
peers.
I'm looking for input on this point. Should we allow all vouchers to be
from the same module and rely on review or other mechanisms to ensure
code quality, consistency, etc? Or should we require vouchers to be
from at least two modules? Some other clever idea?
>
>> 4. Criteria for CVS Access
>>
>> -- In the old world the criteria was understanding of Mozilla
>> architecture and effect of one’s changes on others. The current
>> enumerated items probably need updating.
>> -- Are you proposing something different? Your suggestion sounds
>> more like “can fix bustage” than a focus on the breath of the code.
>> This may be appropriate, no value judgement intended (at least not yet!).
>
> I don't think a broad understanding of the entire codebase is required
> for the type of work many people are doing. One needs to know enough to
> understand impact of their changes, but that amount of knowledge will
> vary broadly depending on the area of code someone is working within.
>
Tempting -- one "should know enough to know enough". I'm inclined to
hope for a bit more here. Ideas welcome. If no one responds, I'll
start making things up :-)
mitchell
Mitchell Baker
close I think -- many thanks to mconnor for getting this going.
Mike Connor wrote:
>
>> 3. Breath of Vouchers
>>
>> -- In the old world the use of super-reviewers required an
>> understanding of interactions between modules and what we called
>> “integration”; a short-hand for some set of coding practices required
>> to work well with other modules.
>> -- In the proposed new world ll 3 vouchers could be from the same module
>
> Correct. For non-SR modules this is actually a higher bar than
> currently exists.
I understand the motivation here. I think this results in people being
able to check code into open modules (say Gecko) without any one of the
Gecko folks or super-reviewers having been involved in granting CVS
access. This may be OK -- all code needs review from module owners or
peers.
I'm looking for input on this point. Should we allow all vouchers to be
from the same module and rely on review or other mechanisms to ensure
code quality, consistency, etc? Or should we require vouchers to be
from at least two modules? Some other clever idea?
>
>> 4. Criteria for CVS Access
>>
>> -- In the old world the criteria was understanding of Mozilla
>> architecture and effect of one’s changes on others. The current
>> enumerated items probably need updating.
>> -- Are you proposing something different? Your suggestion sounds
>> more like “can fix bustage” than a focus on the breath of the code.
>> This may be appropriate, no value judgement intended (at least not yet!).
>
> I don't think a broad understanding of the entire codebase is required
> for the type of work many people are doing. One needs to know enough to
> understand impact of their changes, but that amount of knowledge will
> vary broadly depending on the area of code someone is working within.
>
Tempting -- one "should know enough to know enough". I'm inclined to