Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Revised Getting CVS Access doc

2 views
Skip to first unread message

Mitchell Baker

unread,
Mar 28, 2007, 12:19:44 AM3/28/07
to
The old doc is at
http://www.mozilla.org/hacking/getting-cvs-write-access.html
for those who want to check, compare etc.

I've included below a draft of the new policy as it would appear in a
revised version of the document reference above. Note that I excluded
the "logistics" section here; it barely changes and the meet is below.
It will be included in the final, complete version.

Comments welcome.

mitchell


General


To get CVS write access, you basically need to demonstrate that you know
what you're doing. You'll need to demonstrate competence with the code
you're working with, other Mozilla code you might affect with your work,
Mozilla check-in, build and related processes (like watching the builds
after you’ve checked in code to make sure you haven't broken something)
and basic good coding practices. It also helps if you can leap tall
buildings in a single bound. You'll need to demonstrate this to a set of
people who are willing We judge this by peer review, code review and our
special X-ray vision.In all cases you need to demonstrate you know
what's going on, find a set of people who have sdeuquate authority and
will vouch for your competence, and complete and sign a CVS Contributor
Form. You'll start the process of getting CVS access by submitting good
patches and having them reviewed by the module owner and other
appropriate people. When you think you have submitted patches that
demonstrate the criteria above, you can start the process of obtaining
write access to the source tree. This entire process applies to everyone
who wants write-access to the Mozilla source repository. Employment with
any particular entity (including the Mozilla Foundation) does not change
the need to comply with this policy.

General Rule

The General Rule for write access to the Mozilla source tree is:

1. two vouchers and one super-reviewer must approve your request in
writing (through Bugzilla).

2. The super-reviewer must be from outside the modules in which one has
been working.

3. One of the 3 people approving your access must not be employed by the
same entity as ts the person seeking source code write access.


Voucher Approvals

You need two vouchers. Your vouchers should be the owner or a peer of a
module or modules to which you've been submitting patches. Each vouche
rmust already has CVS commit access and be confident enough in you to be
associated with your check-ins. Your vouchers are responsible for your
bustages in the unfortunate event that you break the tree and leave;
They are resonsible for making sure you know and follow the rules in
general, act promptly to fix regressions, are aware of and respect tree
closures, etc . The vouchers' responsibility extends for three months
after you are granted CVS access. If you've lived in the tree without
significant issues for three months, we assume you're ready to stand on
your own. If somehow there are persistent problems during the first
three months the vouchers have the authority to revoke access during
this period. Vouching is a big responsibility, so people will make this
commitment only after due consideration. A voucher who helps people who
aren't prepared get access to the source tree will find his or her own
credibility suffers as well.

Super-Reviewer approval

If your code is in one or more modules that are included in Firefox,
Thunderbird or the Seamonkey suite then you also need the approval of a
"super-reviewer." Super-reviewers are a small set of contributors who
have, and are known to have, a wide-angle view of the codebase, and are
particularly acute at identifying cross-module issues, integration
issues and other issues beyond the ability of the patch to resolve the
specified issue. More information n super-review and a list of
super-reviewers can be found here.


Scope of Review for Approval

A nominee should have demonstrated both technical chops and an
understanding of Mozilla workstyle (awareness of tree closures,
regressions and bustage processes, good citizenship, responds well to
code reviews, makes changes when appropriate, etc.)

A nominee's code ought demonstrate that s/he has encountered complex
topics and handled them well. It is probably not enough to demonstrate
code that does nothing inappropriate.Currently, it seems unlikely that
this would be demonstrated in less than 3 or 4 good-sized patches. This
process takes a while, so a new participant should anticipate a period
during which the response is ``could well be approved, but I don't have
enough info yet.'' Keep producing good patches and this period will pass.

Here are examples of the types of things the vouchers and
super-reviewers will look for.

-code quality
-- does the proposed committer's code solve the problem it was
intended to? does it do so well? does the code solve an underlying
problem rather than fix a symptom?
-- understanding of relevant aspects of Mozilla architecture; the
definition of "relevant" will depend on the area(s) in which one works.
-- understanding when one's code affects other modules and needs
input beyond one's own area of expertise


- workstlye
--- understands and respects tree rules and related processes
--- availability to deal with issues in one's checkins
--- addresses review comments responsibly


- experience
-- should be a set of good-sized patches adequate to judge quality
issues; and
-- should have a track record that demonstrates other criteria --
at least a couple of months of active hacking that meets the other criteria

Gervase Markham

unread,
Mar 28, 2007, 5:59:11 AM3/28/07
to
Mitchell Baker wrote:
> To get CVS write access, you basically need to demonstrate that you know
> what you're doing. You'll need to demonstrate competence with the code
> you're working with, other Mozilla code you might affect with your work,
> Mozilla check-in, build and related processes (like watching the builds
> after you’ve checked in code to make sure you haven't broken something)
> and basic good coding practices. It also helps if you can leap tall
> buildings in a single bound. You'll need to demonstrate this to a set of
> people who are willing We judge this by peer review, code review and our
> special X-ray vision.

I think some text was either accidentally inserted or is missing here.

> 1. two vouchers and one super-reviewer must approve your request in
> writing (through Bugzilla).
>
> 2. The super-reviewer must be from outside the modules in which one has
> been working.

This has switched from second to third person. "in which you have been"?

> 3. One of the 3 people approving your access must not be employed by the
> same entity as ts the person seeking source code write access.

Or, more simply, "as you"?

> You need two vouchers. Your vouchers should be the owner or a peer of a
> module or modules to which you've been submitting patches.

Do we have a significant number of modules which are either unowned or
unpeered, such that this might be more of a problem for people working
in one area than another?

> Each vouche
> rmust already has CVS commit access and be confident enough in you to be
> associated with your check-ins. Your vouchers are responsible for your
> bustages in the unfortunate event that you break the tree and leave;
> They are resonsible for making sure you know and follow the rules in
> general, act promptly to fix regressions, are aware of and respect tree
> closures, etc .

Small logistical point: there are two vouchers. Which one gets dragged
out of bed at 3am if the new person breaks the tree and can't be
contacted? :-)

Gerv

Axel Hecht

unread,
Mar 28, 2007, 7:03:21 AM3/28/07
to
Mitchell Baker wrote:

<...>


>
> Voucher Approvals
>
> You need two vouchers. Your vouchers should be the owner or a peer of a
> module or modules to which you've been submitting patches. Each vouche
> rmust already has CVS commit access and be confident enough in you to be
> associated with your check-ins. Your vouchers are responsible for your
> bustages in the unfortunate event that you break the tree and leave;
> They are resonsible for making sure you know and follow the rules in
> general, act promptly to fix regressions, are aware of and respect tree
> closures, etc . The vouchers' responsibility extends for three months
> after you are granted CVS access. If you've lived in the tree without
> significant issues for three months, we assume you're ready to stand on
> your own. If somehow there are persistent problems during the first
> three months the vouchers have the authority to revoke access during
> this period. Vouching is a big responsibility, so people will make this
> commitment only after due consideration. A voucher who helps people who
> aren't prepared get access to the source tree will find his or her own
> credibility suffers as well.

I'd expect us to be able to revoke CVS access at any time, and
independent of timing, only with good reasons. The three months time
frame sounds like a contributor could vandalize the tree after 100 days
of sanity and nobody would or could do anything about it.

How about just adding that CVS access can be removed when actions of the
new contributor conflict with the criteria set forth in the remaining doc?

As I assume that vouchers would go to this document for guidance on what
they're expected to do, too, so it might be good to add that having
someones CVS access removed that you vouched for impacts your ability to
vouch in the future. I guess it does, whether we make that official
policy or not.

Reading this with l10n accounts in my head, I do think that the General
paragraph applies nicely, we'll need to do some further discussions on
how the rest applies.

As there are a bunch of terms in this document which folks are supposed
to know, should those be part of the glossary on MDC?

Axel

Robert Kaiser

unread,
Mar 28, 2007, 12:33:38 PM3/28/07
to
Mitchell Baker schrieb:

> Super-Reviewer approval
>
> If your code is in one or more modules that are included in Firefox,
> Thunderbird or the Seamonkey suite then you also need the approval of a
> "super-reviewer."

Nit: It's "SeaMonkey", with a capital "M" ;-)

Should we somewhere place what specifically the sr needs to review when
giving his sr to the application? I fear that we come into the situation
where people have only been working in one module (esp. if it's an
app-specific one like the suite) and all super-reviewers from other
areas might say "I have never reviewed any code of you, so I can't sr
your request". Could we point those sr's to a "but you just need to ...
- my record of patches is at <URL>", or do we think that anyone who gets
to be a sr knows this anyways / anyone applying for VCS access will know
what to ask specifically?

Robert Kaiser

Mitchell Baker

unread,
Mar 28, 2007, 4:31:27 PM3/28/07
to
Gervase Markham wrote:
> Mitchell Baker wrote:
>> To get CVS write access, you basically need to demonstrate that you
>> know what you're doing. You'll need to demonstrate competence with the
>> code you're working with, other Mozilla code you might affect with
>> your work, Mozilla check-in, build and related processes (like
>> watching the builds after you’ve checked in code to make sure you
>> haven't broken something) and basic good coding practices. It also
>> helps if you can leap tall buildings in a single bound. You'll need to
>> demonstrate this to a set of people who are willing We judge this by
>> peer review, code review and our special X-ray vision.
>
> I think some text was either accidentally inserted or is missing here.
> Yup. a set of people who are willing *to be associated with your
check-ins.* We judge this by

>> peer review, code review and our special X-ray vision.

>> 1. two vouchers and one super-reviewer must approve your request in

>> writing (through Bugzilla).
>>
>> 2. The super-reviewer must be from outside the modules in which one
>> has been working.
>
> This has switched from second to third person. "in which you have been"?

right, will fix


>
>> 3. One of the 3 people approving your access must not be employed by
>> the same entity as ts the person seeking source code write access.
>
> Or, more simply, "as you"?

right.


>
>> You need two vouchers. Your vouchers should be the owner or a peer of
>> a module or modules to which you've been submitting patches.
>
> Do we have a significant number of modules which are either unowned or
> unpeered, such that this might be more of a problem for people working
> in one area than another?

This is pretty close to the old rule, where the voucher at least was
supposed to be the owner of the module to which one was submitting patches.


>
>> Each vouche rmust already has CVS commit access and be confident
>> enough in you to be associated with your check-ins. Your vouchers are
>> responsible for your bustages in the unfortunate event that you break
>> the tree and leave; They are resonsible for making sure you know and
>> follow the rules in general, act promptly to fix regressions, are
>> aware of and respect tree closures, etc .
>
> Small logistical point: there are two vouchers. Which one gets dragged
> out of bed at 3am if the new person breaks the tree and can't be
> contacted? :-)
>

Yes, let's leave that to discretion.

> Gerv

Mike Shaver

unread,
Mar 28, 2007, 4:33:08 PM3/28/07
to Axel Hecht, gover...@lists.mozilla.org
On 3/28/07, Axel Hecht <l1...@mozilla.com> wrote:
> I'd expect us to be able to revoke CVS access at any time, and
> independent of timing, only with good reasons. The three months time
> frame sounds like a contributor could vandalize the tree after 100 days
> of sanity and nobody would or could do anything about it.

Yeah, it's a little strange, I wonder if it's better for the voucher
to be asked at the end of the 3 months (omg! so unfair to people who
start in February!) whether they believe that the new contributor can
stand on their own or some such, since it's meant to be a probationary
period.

I think the vouchers have the _responsibility_, not just authority, to
have CVS access disabled if they believe that one of their vouchees
isn't living up to our standards.

> How about just adding that CVS access can be removed when actions of the
> new contributor conflict with the criteria set forth in the remaining doc?

Not just new, IMO. If I screw up badly enough, my access should be
taken away. I think we may want to expire CVS access as well, over
time, since someone who has gone dark from the project for half a year
may not be current on our policies and expectations ("what do you mean
I need to follow the Firefox tinderbox now?").

> As I assume that vouchers would go to this document for guidance on what
> they're expected to do, too, so it might be good to add that having
> someones CVS access removed that you vouched for impacts your ability to
> vouch in the future. I guess it does, whether we make that official
> policy or not.

Agreed that making this more explicit would be good.

I generally quite like what I see here, though -- a big improvement.

Mike

Mitchell Baker

unread,
Mar 28, 2007, 4:51:47 PM3/28/07
to

glad to hear that this feels workable for localization.

On the revoking CVS access, we should probably have a policy for this in
general. In this document I only wanted to note that the vouchers, who
are associated with new committers, should have ability to do this for
the first 90 days.

I've opened a separate bug for a policy about ending CVS access: 375736.

mitchell

Gervase Markham

unread,
Mar 29, 2007, 5:54:41 AM3/29/07
to
Mike Shaver wrote:
> Not just new, IMO. If I screw up badly enough, my access should be
> taken away. I think we may want to expire CVS access as well, over
> time, since someone who has gone dark from the project for half a year
> may not be current on our policies and expectations ("what do you mean
> I need to follow the Firefox tinderbox now?").

If we expire it in this way, I think it would be reasonable to have a
lower barrier for readmittance. After all, people don't (generally) get
less competent as they get older.

Perhaps even the requirement to email to get your account re-enabled
would give the opportunity to send a reply saying, "Sure, but please
reacquaint yourself with current procedures as outlined <a>here</a>."

Gerv

Robert Kaiser

unread,
Mar 29, 2007, 8:28:06 AM3/29/07
to
Mike Shaver schrieb:

> I think we may want to expire CVS access as well, over
> time, since someone who has gone dark from the project for half a year
> may not be current on our policies and expectations ("what do you mean
> I need to follow the Firefox tinderbox now?").

That might be a good idea for people who really don't care about their
CVS access any more. I think it shouldn't be too hard to get a list of
the last commits of contributors out of bonsai. If they haven't done any
commit in the last X months, we could send them some automated mail
asking them if they still need their account. If the reply is "no" or
they don't reply within a reasonable timeframe, the account should
expire or get deactived in some way.
This way we could clean up old accounts and still not harm anyone.

Robert Kaiser

Mike Connor

unread,
Mar 29, 2007, 10:28:40 AM3/29/07
to Robert Kaiser, gover...@lists.mozilla.org

That adds essentially useless overhead, and I don't think I care so
much about someone basically absent for six months having to get
their account reactivated. There's no harm in requiring someone
coming back to the project having to file a bug or email a specific
group (i.e. SRs or some appropriate developer group). There is
potential harm in someone stringing along with active access without
actually being active, namely the danger of having people who are
truly out of the loop checking in, etc.

-- Mike

Robert Kaiser

unread,
Mar 29, 2007, 10:54:07 AM3/29/07
to
Mike Connor schrieb:

I still think no account should be revoked without a prior warning. That
would sound pretty harsh to me.

Robert Kaiser

Mike Connor

unread,
Mar 29, 2007, 11:14:03 AM3/29/07
to gover...@lists.mozilla.org

The warning is "if you stop using your account for six months, access
will be disabled"

-- Mike

Mitchell Baker

unread,
Mar 29, 2007, 12:39:30 PM3/29/07
to

Yes, something like this makes sense. Or it might be an "inactive"
status. although i suppose if someone is gone long enough his or her
ynderstanding of the code could get pretty stale.

mitchell

Message has been deleted

Mitchell Baker

unread,
Mar 29, 2007, 7:13:09 PM3/29/07
to
Don't know. I am known for having a range of odd issues with all my
software. And even recently, with the new mac machines, which have not
been reliable for me

mitchell

Peter Weilbacher wrote:


> On Wed, 28 Mar 2007 20:31:27 UTC, Mitchell Baker wrote:
>
>> Gervase Markham wrote:
>>> Mitchell Baker wrote:
>>>> It also
>>>> helps if you can leap tall buildings in a single bound. You'll need to
>>>> demonstrate this to a set of people who are willing We judge this by
>>>> peer review, code review and our special X-ray vision.
>>> I think some text was either accidentally inserted or is missing here.
>>> Yup. a set of people who are willing *to be associated with your
>> check-ins.* We judge this by
>> >> peer review, code review and our special X-ray vision.
>

> Is that a weird bug in the old TB version that you are using so that it
> happened again?

Christian Biesinger

unread,
Mar 29, 2007, 8:16:34 PM3/29/07
to
Mitchell Baker wrote:
> 2. The super-reviewer must be from outside the modules in which one has
> been working.

What does this mean exactly? sr isn't supposed to be module-specific.

If you mean the modules that the sr list shows for the super-reviewers,
is it enough if the sr has other modules than the ones in question? Or
does it have to be a super-reviewer that has nothing to do with the
modules that the person has been working on?

Axel Hecht

unread,
Mar 30, 2007, 5:28:28 AM3/30/07
to

I personally think that six months is a tad short, and we might want to
take other activity measures in there, too, like reviews, or even just
bug traffic.

I don't think that any piece of code we have changes that badly on a
continuous basis that ones knowledge dies after 6 months. And for those
changes that happen, there's still a reviewer that should be up-to-date
on that code.

We could ask folks though if they still want their account or if they'd
be better off with check-in-buddies doing that for them on occasion.

Axel

Simon Paquet

unread,
Apr 11, 2007, 6:38:02 PM4/11/07
to
Mitchell Baker wrote on 28. Mar 2007:

> Super-Reviewer approval
>
> If your code is in one or more modules that are included in
> Firefox, Thunderbird or the Seamonkey suite then you also
> need the approval of a "super-reviewer."

Do we need product names here? If a project decides to move to
another review system (Calendar Project changed its review system
twice in the last 2 years), this document will have to be updated
every time.

And which rules do apply if two projects with different rules merge?

For example, the SeaMonkey or Thunderbird folks might very well
decide in the future to build the Lightning extension by default.

This may not even require the explicit approval of Calendar
developers (although we certainly hope, that we would be asked),
if the relevant changes would only need some makefile magic in
TB or SM.

Would that lead to a situation

1. where aspiring Calendar hackers would need an additional sr to
gain cvs access, where before only two vouchers were necessary?
2. where an automatic change of review rules (sr now required)
would come into effect for Calendar code?


--
Simon Paquet
Sunbird/Lightning website maintainer
Project website: http://www.mozilla.org/projects/calendar
Developer blog: http://weblogs.mozillazine.org/calendar

Mitchell Baker

unread,
Apr 11, 2007, 7:17:13 PM4/11/07
to
Simon Paquet wrote:
> Mitchell Baker wrote on 28. Mar 2007:
>
>> Super-Reviewer approval
>>
>> If your code is in one or more modules that are included in Firefox,
>> Thunderbird or the Seamonkey suite then you also need the approval of
>> a "super-reviewer."
>
> Do we need product names here? If a project decides to move to
> another review system (Calendar Project changed its review system
> twice in the last 2 years), this document will have to be updated
> every time.
>
> And which rules do apply if two projects with different rules merge?
>
> For example, the SeaMonkey or Thunderbird folks might very well
> decide in the future to build the Lightning extension by default.
>
> This may not even require the explicit approval of Calendar
> developers (although we certainly hope, that we would be asked),
> if the relevant changes would only need some makefile magic in
> TB or SM.
>
> Would that lead to a situation
>
> 1. where aspiring Calendar hackers would need an additional sr to
> gain cvs access, where before only two vouchers were necessary?
> 2. where an automatic change of review rules (sr now required)
> would come into effect for Calendar code?
>
>
Well, Stuart argues that these rules should apply to all code in Mozilla
source repositories. That would be a change for things like webtools,
which have historically been an exception. I am personally leaning
towards Stuart's view, but we haven't had a discussion of that yet.
And the old policy had a product name -- "in process with application
suite." So I took the projects that most closely match the old rule.
This is interim until we have the discussion about scope.

As to review policy, do you mean code review policy? If so, that
linkage has been broken in the new document. The old doc said that if
your code needed super-review, one needed super-review for CVS access.
You're quite right that this tied CVS policy to code review policy.
That's no longer the case.

As to the question of two projects merging, this would be solved by
Stuart's suggestion -- all code would follow the same rule. If we don't
end up with something like Stuart's proposal then these sort of issues
might come up.

And yes, I agree that policies will change. There may well be settings
where new contributors to Calendar, or any other project, find
themselves working under a different policy for access than contributors
who joined earlier or later.

Mitchell

Gervase Markham

unread,
Apr 12, 2007, 4:44:35 AM4/12/07
to Mitchell Baker
Mitchell Baker wrote:
> Well, Stuart argues that these rules should apply to all code in Mozilla
> source repositories. That would be a change for things like webtools,
> which have historically been an exception. I am personally leaning
> towards Stuart's view, but we haven't had a discussion of that yet.

When and where do you think might be an appropriate point to talk about
this issue? Getting the right people together might take a little
warning because, by the very nature of the proposal, it most affects
those on the edges of the project.

Gerv

Simon Paquet

unread,
Apr 12, 2007, 7:12:21 AM4/12/07
to
Mitchell Baker wrote on 12. Apr 2007:

> Well, Stuart argues that these rules should apply to all code in
> Mozilla source repositories. That would be a change for things
> like webtools, which have historically been an exception. I am
> personally leaning towards Stuart's view, but we haven't had a
> discussion of that yet.

I disagree with Stuart here. This sounds a lot like consistency for
consistency's sake. One rule should stand above all others "If it
ain't broke, then don't fix it".

In can only speak for the Calendar module (where I have cvs access).
And there's nothing broken there. We've had no problems in the past
with just the requirement, that to gain cvs access, you needed
module owner approval.

Personally I have no problem with the new proposal, requiring two
vouchers for modules outside the sr-scope. It would be no problem
for the Calendar module now, as we have more than a handful of peers
now.

But I still remember the days, when we just had one module owner and
no peers. We got our first peer back then, because mvl (who already
had cvs access) joined the project.

So please remember, that not every project is as big as Firefox.

> And the old policy had a product name -- "in process with
> application suite." So I took the projects that most closely
> match the old rule. This is interim until we have the discussion
> about scope.

I'm personally still wondering why the SeaMonkey guys still bother
with the sr-restriction, now that they have mainly front-end code
to maintain. But of course it is their project and they can do
whatever they want to ;-)

> As to review policy, do you mean code review policy? If so, that
> linkage has been broken in the new document. The old doc said
> that if your code needed super-review, one needed super-review
> for CVS access. You're quite right that this tied CVS policy to
> code review policy. That's no longer the case.

Ok, thanks for the clarification. The current wording still implies
this connection to be the case. I think that it wouldn't hurt to
clearly state, that one is not tied to the other.

Robert Kaiser

unread,
Apr 12, 2007, 1:03:57 PM4/12/07
to
Simon Paquet schrieb:

> I'm personally still wondering why the SeaMonkey guys still bother
> with the sr-restriction, now that they have mainly front-end code
> to maintain. But of course it is their project and they can do
> whatever they want to ;-)

Actually, we're using sr as something similar to ui-review and design
review nowadays. And seeing how much useful changes are turned up by sr
all the time, we feel it is better to go with that instead of worse code :)

Robert Kaiser

Mitchell Baker

unread,
Apr 16, 2007, 5:51:45 PM4/16/07
to

Gerv

I think this newsgroup is the place. I personally would like to think
through some issues related to super-review next. I have a feeling
working through that a bit might make other things easier. But I'm not
going to suggest that people don't discuss this now if there is interest.

ml

Gavin Sharp

unread,
Jun 19, 2007, 1:35:20 AM6/19/07
to Mitchell Baker, Christian Biesinger

I think it's important that we get clarification on what exactly
requirement #2 means.

First, I think we need an answer to biesi's question about the meaning
of "must be from outside the modules". As biesi mentions, SR has never
been module-specific, so the it's not clear what exactly this
requirement implies. Based on discussion on IRC it seems that people
have taken "modules" to mean either "the areas listed next to the SR's
name at http://www.mozilla.org/hacking/reviewers.html", or "the list of
modules for which the super-reviewer is a peer/owner". Clarification on
what is meant by "modules" in this context would be appreciated.

Second, assuming one of those interpretations is correct, I'm not sure I
understand the motivation behind the requirement. Why must the super
reviewer not specialize in the code that the applicant has been most
involved in? The only reason I see for this kind of limitation would be
to avoid bias on the super-reviewers part, but I'd like to think that we
can trust our super-reviewers to provide objective evaluations of CVS
access applicants, regardless of their involvement with the code that
the applicant is principally contributing to. In fact, if taken
literally this rule seems like it would make it more difficult for an
applicant that's worked on many different parts of the code to find a
suitable super reviewer. I'm sure that wasn't the intent, but I think
the ambiguity is troublesome.

Past experience has shown that getting super-reviewers to vouch for you
is one of the hardest parts of getting CVS access. I don't think that
that's necessarily a bad thing, in and of itself; I think the
super-review requirement is an important one. However, super-reviewers
tend to be busy, and limiting the set of valid super-reviewers to those
that haven't seen or dealt with your code, and therefore are unlikely to
be familiar with your work, seems like a limitation whose costs outweigh
the benefits.

Gavin

Gavin Sharp

unread,
Jun 19, 2007, 1:36:35 AM6/19/07
to Mitchell Baker, Christian Biesinger

I think it's important that we get clarification on what exactly

Mitchell Baker

unread,
Jul 3, 2007, 11:41:38 AM7/3/07
to
You're right, we should resolve this. I missed Christian's message the
first time. I mean, I'm sure I saw it, but it didn't register correctly
as something to figure out.

First, as to the requirement for this: I originally wrote:

"This breath requirement is intended as a check on group optimism."
Someone else asked what this meant. Gerv responded more eloquently than
I (see his messsage dated 3/12 in this thread):

"I believe Mitchell is saying that if a small group of people are
working together on code (e.g. Fred, an owner, Jake, a peer and Bill, a
third person without CVS access), then it's good to have someone outside
the small group assess Bill's suitability if he applies. It's possible
that those close to Bill will have "group optimism" - that is, an
artificially high view of Bill's work quality because they work closely
together. So it helps to have someone with more distance look at the
application.

Gerv"


The requirement that SRs be "outside the module" was intended to provide
this breath. The criticism that SRs aren't tied to modules and this
isn't clear is completely valid.

Christian asked about the intent:

>>, is it enough if the sr has other modules than the
>> ones in question?

Yes, this was the idea.

>>Or does it have to be a super-reviewer that has
>> nothing to do with the modules that the person has been working on?

No, this was not at all the idea. The requirement should have been
written as something like:


"2. The super-reviewer must have experience working in at least one
module other than those in which the person requesting CVS access has
been working. "

I would like to make a change like this to the policy now. Please let
me know if you think this is a bad idea.


Remaining questions:

Overall Role of Super-Review. I did a bit of research on this a while
back, and then got pulled into something else. I'll try to get this
started soon

CVS Access Difficulties. I know that there are some modules (KaiRo has
described SeaMonkey in this thread) which find the SR review difficult
because they work in areas rather separate from most of the remaining
SRs. This still requires an SR to look at people writing SeaMonkey
code, etc.

Does clarifying the intent as above make things any easier?

Mitchell

Robert Kaiser

unread,
Jul 3, 2007, 11:58:34 AM7/3/07
to
Mitchell Baker schrieb:

> CVS Access Difficulties. I know that there are some modules (KaiRo has
> described SeaMonkey in this thread) which find the SR review difficult
> because they work in areas rather separate from most of the remaining
> SRs. This still requires an SR to look at people writing SeaMonkey
> code, etc.
>
> Does clarifying the intent as above make things any easier?

Sure, as e.g. in the SeaMonkey case noted above, I think all
super-reviewers that do lots of work on SeaMonkey (biesi, Neil, jag)
also have quite good experience in other areas.
I think this might also be true for at least one sr in other modules, so
I think it helps everyone.

Robert Kaiser

Mitchell Baker

unread,
Jul 3, 2007, 12:12:29 PM7/3/07
to

Excellent!

ml

Mike Connor

unread,
Jul 3, 2007, 12:12:04 PM7/3/07
to Mitchell Baker, gover...@lists.mozilla.org
Mitchell Baker wrote:
> "This breath requirement is intended as a check on group optimism."
> Someone else asked what this meant. Gerv responded more eloquently than
> I (see his messsage dated 3/12 in this thread):
>
> "I believe Mitchell is saying that if a small group of people are
> working together on code (e.g. Fred, an owner, Jake, a peer and Bill, a
> third person without CVS access), then it's good to have someone outside
> the small group assess Bill's suitability if he applies. It's possible
> that those close to Bill will have "group optimism" - that is, an
> artificially high view of Bill's work quality because they work closely
> together. So it helps to have someone with more distance look at the
> application.
>
> Gerv"
>
>
> The requirement that SRs be "outside the module" was intended to provide
> this breath. The criticism that SRs aren't tied to modules and this
> isn't clear is completely valid.
>
Having thought about this a great deal lately, I'm going to strongly
assert that SRs must be able to elevate their decision-making, and
eschew the trap of group optimism by the time they're entrusted with the
role of SR. IMO, if I am unable to fairly evaluate someone working on
/browser, I have no right to be listed as an SR, since that demonstrates
an inability to separate the needs of the module from the needs of the
overall codebase. Having someone of greater experience and demonstrated
broad-based judgement is a fair requirement, but a truly external SR is
a little bit of an odd bar. biesi has fortunately spared the cycles to
verify and vouch for a number of the front end CVS access grants under
this policy, but I'm not sure that its necessary or correct to ask this
of him.

> Christian asked about the intent:
>
> >>, is it enough if the sr has other modules than the
> >> ones in question?
>
> Yes, this was the idea.
>

What does it mean to have other modules? AIUI, the modules are
suggestions, any SR can SR for any module, though in practice it makes
more sense to have preferred SRs for specific areas.

> "2. The super-reviewer must have experience working in at least one
> module other than those in which the person requesting CVS access has
> been working. "
>
> I would like to make a change like this to the policy now. Please let
> me know if you think this is a bad idea.
>

So, really, I don't think anyone qualifies as an SR unless they've done
enough in different parts of the codebase to establish themselves as
being outside of a single area. If you are working in multiple areas,
and the requestee happens to match all of those areas, I think group
optimism is out of play, since in theory you'll have a much broader
group of module owners and peers working with the individual.

> CVS Access Difficulties. I know that there are some modules (KaiRo has
> described SeaMonkey in this thread) which find the SR review difficult
> because they work in areas rather separate from most of the remaining
> SRs. This still requires an SR to look at people writing SeaMonkey
> code, etc.
>
> Does clarifying the intent as above make things any easier?
>

I think that simply requiring an SR as a voucher solves this problem,
since I trust Neil and jag to do the right thing when evaluating
SeaMonkey hackers.

-- Mike

Axel Hecht

unread,
Jul 3, 2007, 1:38:42 PM7/3/07
to
Mitchell Baker wrote:
> You're right, we should resolve this. I missed Christian's message the
> first time. I mean, I'm sure I saw it, but it didn't register correctly
> as something to figure out.
>
> First, as to the requirement for this: I originally wrote:
>
> "This breath requirement is intended as a check on group optimism."
> Someone else asked what this meant. Gerv responded more eloquently than
> I (see his messsage dated 3/12 in this thread):
>
> "I believe Mitchell is saying that if a small group of people are
> working together on code (e.g. Fred, an owner, Jake, a peer and Bill, a
> third person without CVS access), then it's good to have someone outside
> the small group assess Bill's suitability if he applies. It's possible
> that those close to Bill will have "group optimism" - that is, an
> artificially high view of Bill's work quality because they work closely
> together. So it helps to have someone with more distance look at the
> application.
>

I'm curious if we need to map this to l10n accounts.

I'd be facing two sides of the medal here, linguistic and localization
skills on the one hand, and technical skills on the other.

We have hardly any angle on the linguistic part, we could have an angle
on the technical side, like, can the localizer do on-focus patches, use
compatible tools with the rest of the gang, use patch flags, and
ultimately, use CVS.

Would that make sense?

The downside here is, we can't really assess the patch work abilities
before we have code, so the initial author of a localization would face
a different technical review than a peer coming in later. I wonder if
that's OK.

Axel

Mitchell Baker

unread,
Jul 3, 2007, 2:09:12 PM7/3/07
to
Mike

that's a good point. Maybe an "SR" designation means we've already
decided this person has the expertise and has proven s/he can and will
consider the breadth of the overall codebase.

Interested in whether others feel comfortable with this.

mitchell

Gervase Markham

unread,
Jul 4, 2007, 4:56:36 AM7/4/07
to
Mitchell Baker wrote:
> that's a good point. Maybe an "SR" designation means we've already
> decided this person has the expertise and has proven s/he can and will
> consider the breadth of the overall codebase.
>
> Interested in whether others feel comfortable with this.

I am. One can flip it round to see it another way: if we feel and SR is
approving inappropriate or under-skilled people, we might need to
reconsider their holding the position of SR.

Gerv

Gavin Sharp

unread,
Jul 5, 2007, 10:03:53 PM7/5/07
to
Mitchell Baker wrote:
> Maybe an "SR" designation means we've already decided this person has
> the expertise and has proven s/he can and will consider the breadth of
> the overall codebase.

Indeed, that was the point I was trying to make in my earlier post. I
trust that all of our SRs can make fair, unbiased decisions on CVS
access requests, regardless of how "close" they are to the applicant's
main area of focus.

Gavin

Mitchell Baker

unread,
Jul 6, 2007, 12:31:09 PM7/6/07
to


We know that super-reviewers are people with both great integrity and
great skill in the Mozilla world. At the same time, it’s easy for even
great people to be influenced by a group dynamic. (I recall vividly a
couple of times when Brendan felt I had been overly influenced by some
group or other. It wasn’t fun, but I was later very happy he had
challenged me.)

So I try to build organizational structures that rely on great people,
and also provide input from different perspectives to help those people
make their best decisions. Thus the interest in getting the viewpoint
from outside the module in question.

Mitchell

0 new messages