CVS Access proposal (aka Round 3)
Mitchell Baker
poking around I did to assure myself that the role of super-review and
the identity of super-reviewers is likely to continue. (Bug 366103 is
the tracking bug for the role of super-review. We'll have that
discussion publicly soon. But for now I'm confident enough that we
won't eliminate super-reviewers to keep them in the proposal below.)
Most significant changes from mconnor's suggestions are requiring one
SR, and the SR to be from somewhere else in the codebase; and adding
something explicit about code quality. Thanks to mconnor for doing most
of the hard work.
Comments welcome. If I don't hear significant concerns, I'll turn to
getting the official documents on the website updated to reflect the
content below.
ml
Proposal
1. Number and Identity of Required Sponsors
-- One needs three sponsors: two vouchers and a super-reviewer.
-- Each of the two vouchers must be a module owners or peer. They can
be from the same module, and they should be from the module(s) where the
proposed committer committed patches.
-- The third person must be a super-reviewer, and shuldn't be from the
same modules as the two vouchers. This breath requirement is intended
as a check on group optimism.
(The old policy required 1 voucher and 2 or 3 super-reviewers).
2. Role of Vouchers
-- Vouching is a commitment to work with the new committer until he or
she is ready to work indpendantly. This means the vouchers should keep
tabs on check-ins from the new committer and help resolve issues that
might arise, such as awareness of closed trees, untended regressions,
prompt response to bustage, etc.)
-- If some Vouchers are consistently over-optimistic about the people
they vouch for we'll figure out what to do. I'd rather not set up an
elaborate policy about it's clear we need.
3. Criteria for Access
--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
4. Employement of Sponsors
At least one of the three sponsors must not have the same employer as
the proposed committer.
mitchell
Nelson B
> Here's a proposal. It's based on the previous thread, and on a bit of
> poking around I did to assure myself that the role of super-review and
> the identity of super-reviewers is likely to continue.
Mitchell,
Mozilla's source repository houses more than just the source code to
mozilla's FireFox and Thunderbird projects. The rules you proposed may
be perfect for deciding who gets write access to the sources of FF and TB,
but may be inappropriate for some of the other projects.
One area I have in mind is NSS/JSS/NSPR. These projects build by them
selves, stand alone. Mozilla clients depend on them, but they do not
depend on mozilla client software. The criteria for having write
access to those projects must-needs be different. Knowledge of other
parts of the mozilla client source base is not relevant to qualification
to work on NSS/JSS/NSPR. Approval by people who know nothing about NSS
seems irrelevant to deciding the qualifications for NSS write access.
Mozilla's source repository has an access control system at the level of
individual source "modules". People can be granted write access to one
module but not another. Quite a few people who have write access to NSS
sources today do not have those access rights to mozilla modules, and
vice versa.
It seems to me that the things you have proposed as being conditions to
getting a CVS account should actually be conditions of getting write
access to certain of the mozilla project modules, and that conditions
for getting a CVS account should be such that meeting the conditions
for write access to ANY one of the modules should be enough to gt the
account.
--
Nelson B
Mike Connor
On 10-Mar-07, at 11:40 PM, Nelson B wrote:
> One area I have in mind is NSS/JSS/NSPR. These projects build by them
> selves, stand alone. Mozilla clients depend on them, but they do not
> depend on mozilla client software. The criteria for having write
> access to those projects must-needs be different. Knowledge of other
> parts of the mozilla client source base is not relevant to
> qualification
> to work on NSS/JSS/NSPR. Approval by people who know nothing about
> NSS
> seems irrelevant to deciding the qualifications for NSS write access.
NSS/JSS/NSPR has historically been treated as an exception. How we
modify that exception is open for discussion. Webtools falls into
the same category.
Maybe its as simple as allowing a third module owner in place of SR
for a specific set of modules?
> Mozilla's source repository has an access control system at the
> level of
> individual source "modules". People can be granted write access to
> one
> module but not another. Quite a few people who have write access
> to NSS
> sources today do not have those access rights to mozilla modules, and
> vice versa.
Actually, the NSS accounts afaict have access to all of the open
partitions, which is most of the tree. It is NSS that is currently
closed and thus inaccessible.
> It seems to me that the things you have proposed as being
> conditions to
> getting a CVS account should actually be conditions of getting write
> access to certain of the mozilla project modules, and that conditions
> for getting a CVS account should be such that meeting the conditions
> for write access to ANY one of the modules should be enough to gt the
> account.
I think that as we move to Hg we'll want to consider splitting NSS,
webtools, and possibly other (relatively) standalone projects into
their own discrete repositories with their own access controls. This
would separate the legal (agreement etc) aspect of the account
creation separate from access to the various repos. In theory,
despot v2 will allow granting access to different repos from a single
base LDAP account, and from there NSS and webtools and other projects
will be free to define their own policies around access to their own
repos.
In the meantime, however, since much of cvs.m.o is open partitions,
there needs to be a global policy with well-defined exceptions.
-- Mike
Nelson B
> NSS/JSS/NSPR has historically been treated as an exception. How we
> modify that exception is open for discussion. Webtools falls into
> the same category.
Yes, and I don't see why they should continue to be exceptions.
Why should use of mozilla's despot access controls be exceptional?
Why isn't it the rule, the norm?
> Maybe its as simple as allowing a third module owner in place of SR
> for a specific set of modules?
I think not.
How is the owner of a third module relevant to access to NSS?
Or did you mean a third owner of a particular module?
(NSS no longer has 3 module owners.
http://www.mozilla.org/owners.html is overdue for an update.)
>> Mozilla's source repository has an access control system at the level
>> of individual source "modules". People can be granted write access to
>> one module but not another. Quite a few people who have write access to
>> NSS sources today do not have those access rights to mozilla modules,
>> and vice versa.
> Actually, the NSS accounts afaict have access to all of the open
> partitions, which is most of the tree. It is NSS that is currently
> closed and thus inaccessible.
Right. Mozilla devised an access control system for its repository,
which others have put to good use, but which mozilla itself seems not
to be willing to use.
Mike, here's what I see happening. The keepers of the big mozilla
project now want greater access control over their partitions. They
want to raise the bar for admission to their partitions. That's fine.
But they're evidently unwilling to use the despot access control system
that they the devised for this very purpose. So, instead they wish to
impose their new access controls on all partitions, to make it a condition
of getting an account for the entire repository. And to projects that
don't want to accept these particular new access controls, the
suggestion is now being made to move to another repository. :(
>> It seems to me that the things you have proposed as being conditions to
>> getting a CVS account should actually be conditions of getting write
>> access to certain of the mozilla project modules, and that conditions
>> for getting a CVS account should be such that meeting the conditions
>> for write access to ANY one of the modules should be enough to get the
>> account.
> I think that as we move to Hg we'll want to consider splitting NSS,
Move to Hg? What's Hg? Mercury? (I really don't know that this means)
> webtools, and possibly other (relatively) standalone projects into
> their own discrete repositories with their own access controls.
All to avoid mozilla partitions using despot's access controls?
> This would separate the legal (agreement etc) aspect of the account
> creation separate from access to the various repos.
More than that, separate repositories mean separate account creation
rules, and potentially separate agreements. That must be so, else there
is no point in having a separate repository.
> In theory, despot v2 will allow granting access to different repos from
> a single base LDAP account, and from there NSS and webtools and other
> projects will be free to define their own policies around access to their
> own repos.
So, you still want despot to be the access control system, but you just
don't want to use it for mozilla's partition(s) ?
What's the problem with using despot for mozilla's partitions?
It it too cumbersome to use?
Has mozilla's repository been divided into too many partitions, all of
which have the same (presently open) access control rules?
Is the problem that, to impose access controls on mozilla partitions,
it would mean adding each new mozilla account holder to too many
partitions?
One imagines that perhaps all the partitions that share a common
access control list should be collapsed into a single partition.
Or perhaps despot should be enhanced to be able to modify the ACLs
for numerous partitions with a single transaction (single submit).
> In the meantime, however, since much of cvs.m.o is open partitions,
> there needs to be a global policy with well-defined exceptions.
Yes, as long as mozilla's partitions continue to use no ACLs,
mozilla's access repository must be the repository-wide policy. :(
> -- Mike
/Nelson B
Robert Kaiser
> Mike Connor wrote:
>
>> NSS/JSS/NSPR has historically been treated as an exception. How we
>> modify that exception is open for discussion. Webtools falls into
>> the same category.
>
> Yes, and I don't see why they should continue to be exceptions.
> Why should use of mozilla's despot access controls be exceptional?
> Why isn't it the rule, the norm?
Probably because administration easily gets a full-time job, maybe for
more than one person and very confusing if everyone has different
permissions and some people need to do checkins that touch multiple
areas of code...
I know many people who'd be glad to reduce bureaucracy in many areas
instead of adding even more of it due to those detailed access controls...
> Mike, here's what I see happening. The keepers of the big mozilla
> project now want greater access control over their partitions. They
> want to raise the bar for admission to their partitions. That's fine.
Actually, the current goal is more in the other direction, IMHO. We
currently need 3 super-reviewers (plus a voucher) to get access, which
gets a too high bar for what many modules need, as super-reviewers
aren't required any more in many areas and therefore get fewer all the time.
We need rules that even work with a decreased number of super-reviewers
and stronger module-internal ownership of the code. Still, it's probably
a good idea if someone from the SeaMonkey team can check in improvements
to the Lightning extension himself (which may work with SeaMonkey) after
appropriate reviews, or if a Thunderbird developer can check in the
appropriate changes with a shared-mail-backend change for both
Thunderbird and SeaMonkey mail (again, with appropriate reviews).
Exactly those things are when tight access controls start to bite you.
Every person getting CVS access signs a form that explains the rules,
we're requiring reviews and good check-in comments across the tree, and
have the possibility of revoking access of people who don't follow the
rules. And then, all this non-restriction of access works quite well, as
I don't remember any malicious misuse in all the years I'm in the
project now.
I know a few people though who originally got access for very
specialized use but have evolved to use it in good ways for much more
than the original purpose - and no bureaucracy needed for that, no time
of some admins wasted to give them additional privileges - and, most
importantly probably, no blocking of development flow for weeks due to
overworked admins who don't find the time to grant that access.
>> I think that as we move to Hg we'll want to consider splitting NSS,
>
> Move to Hg? What's Hg? Mercury? (I really don't know that this means)
Mercurial, actually. A different source-control system, which is said to
suck majorly by folks who actually did use it (anyone been talking to
Flock folks?) - anyways, looks like Mozilla will use it in the future.
And, who knows, maybe we find out that Flock devs were just too stupid
to work with it and it's perfect for us...
> Yes, as long as mozilla's partitions continue to use no ACLs,
> mozilla's access repository must be the repository-wide policy. :(
Which I consider to be a good idea, actually.
Robert Kaiser
Gervase Markham
> Comments welcome. If I don't hear significant concerns, I'll turn to
> getting the official documents on the website updated to reflect the
> content below.
Does this policy cover the entire Mozilla CVS tree? If so, is there a
list of exceptions? For example, some independent teams and projects
have historically had their own rules - e.g. Bugzilla, and (I believe)
NSPR and NSS closer to home.
Similarly, the l10n community has its own rules for admitting localisers
to make changes to specific locales. These are currently outlined here:
http://developer.mozilla.org/en/docs/Create_a_new_localization
under "File a CVS account request bug" and "Submit the localization for
review".
Only one voucher, plus an l10n code review, is required for the
localisation owner; no code review is required for the peers.
Given that, as I understand it, it is technically complex to manage many
CVS access restriction partitions, we have been giving people like
localisers "full access" accounts and using social controls to prevent
errant checkins. Will this approach continue?
It might also tidy up a loose end if we were to explicitly state that
the mozilla.org website CVS tree is not covered.
> 1. Number and Identity of Required Sponsors
>
> -- One needs three sponsors: two vouchers and a super-reviewer.
>
> -- Each of the two vouchers must be a module owners or peer. They can
> be from the same module, and they should be from the module(s) where the
> proposed committer committed patches.
Or rather, "has submitted patches"; they can't have committed them yet :-)
> --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?
That latter is a little harsh as an unqualified criteria; in limited
circumstances, such as fixes which must be limited in scope because the
branch is stable, one wants to fix the symptom rather than address
underlying problems. But perhaps that's too detailed a nuance for this
document.
Hope that's useful,
Gerv
Gervase Markham
>> This breath requirement is intended as a check on group optimism.
>
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
Gervase Markham
> NSS/JSS/NSPR has historically been treated as an exception. How we
> modify that exception is open for discussion. Webtools falls into the
> same category.
Sure. Although I think an active case needs to be made for change,
rather than requiring an active case for the status quo. If there are no
reports of code quality issues in these modules, and if their owners are
happy with the way things are, and if the boundaries between them and
the main codebase are clearly defined, why upset their working practices?
> Actually, the NSS accounts afaict have access to all of the open
> partitions, which is most of the tree. It is NSS that is currently
> closed and thus inaccessible.
Right. There are currently a lot of social controls about who can check
in where. And, as you say, as long as we continue to use CVS, that will
need to continue unless the amount of despot-jockeying is going to rise
substantially.
To be honest, social controls seem to be working well. Do we have any
recent examples of people checking in where they have not been supposed
to, with bad results?
> I think that as we move to Hg we'll want to consider splitting NSS,
> webtools, and possibly other (relatively) standalone projects into their
> own discrete repositories with their own access controls. This would
> separate the legal (agreement etc) aspect of the account creation
> separate from access to the various repos.
Is anyone suggesting that we have separate legal agreements for
committing to NSS as opposed to the main Mozilla codebase? If not, then
the legal agreement will probably just continue to cover all Mozilla
repositories, as it does now (and as the new draft does).
New draft: http://wiki.mozilla.org/Contributor_Form
We would need to assess whether having these splits and extra access
controls helps more than it impedes. I guess that again, data on recent
erroneous CVS checkins would be useful.
Gerv
Gervase Markham
> Mike Connor wrote:
>> I think that as we move to Hg we'll want to consider splitting NSS,
>
> Move to Hg? What's Hg? Mercury? (I really don't know that this means)
The current plan of record for Mozilla 2 (the development which happens
after Firefox 3) is to move the Mozilla source code repository to a new
source code management system called Mercurial, abbreviated Hg.
http://www.selenic.com/mercurial/wiki/
This system was chosen after an evaluation of several possible
alternatives. I haven't used it myself, but apparently it better
supports distributed working and merging and code rearrangement, all of
which are expected to be key features of that phase of the project's
development.
It might be that this decision has not yet been adequately communicated
to some constituencies within the project, such as your own.
Gerv
Gijs Kruitbosch
> <snip>
> Proposal
>
> 1. Number and Identity of Required Sponsors
>
> -- One needs three sponsors: two vouchers and a super-reviewer.
>
> -- Each of the two vouchers must be a module owners or peer. They can
> be from the same module, and they should be from the module(s) where the
> proposed committer committed patches.
>
> -- The third person must be a super-reviewer, and shuldn't be from the
> same modules as the two vouchers. This breath requirement is intended
> as a check on group optimism.
>
> (The old policy required 1 voucher and 2 or 3 super-reviewers).
>
In the old policy, there was also the rule that code modules that did
not require superreview did not require super-reviewers to get someone a
CVS account. See the "Voucher and Super-reviewers" section on
http://www.mozilla.org/hacking/getting-cvs-write-access.html .
What will happen to this? Will those people still need "just" two
vouchers? Maybe keep it like that at least until discussion about the
role/existence of super-reviewers is publicly dealt with?
~ Gijs
Robert Kaiser
> Similarly, the l10n community has its own rules
L10n has its own repository as well, and that one can easily have a
different set of rules.
Robert Kaiser
Robert Kaiser
> Given that, as I understand it, it is technically complex to manage many
> CVS access restriction partitions, we have been giving people like
> localisers "full access" accounts and using social controls to prevent
> errant checkins. Will this approach continue?
Oh, forgot to comment on that in my other post ;-)
We have not given localizers "full access" accounts on the main
mozilla.org repository (with one exception a long time ago), as the L10n
CVS is a different repository. Localizers only get (full) access to that
separate repository.
I think though that the use of social controls instead of real access
control has not only reduced bureaucracy on granting access to CVS, it
also has eased people starting in one area and growing to be
contributors to other parts of the project. With the bar for getting an
account at all being reasonably high (needing 3 good, existing
contributors to be confident of the "newcomer", signing the CVS form,
etc.) we have managed to get only people into CVS access who don't need
much social control, and with the review requirements for every single
checkin and a fairly active community of people watching Bonsai closely,
that ecosystem works quite well. This is a working meritocracy :)
BTW, that one exception who got that "full access" account as a
localizer, with a then-valid low bar for that group, was me, and I guess
I'm no bad example of growing into a contributor to other areas. ;-)
Robert Kaiser
Axel Hecht
> Mitchell Baker wrote:
>> Comments welcome. If I don't hear significant concerns, I'll turn to
>> getting the official documents on the website updated to reflect the
>> content below.
>
> Does this policy cover the entire Mozilla CVS tree? If so, is there a
> list of exceptions? For example, some independent teams and projects
> have historically had their own rules - e.g. Bugzilla, and (I believe)
> NSPR and NSS closer to home.
>
> Similarly, the l10n community has its own rules for admitting localisers
> to make changes to specific locales. These are currently outlined here:
> http://developer.mozilla.org/en/docs/Create_a_new_localization
> under "File a CVS account request bug" and "Submit the localization for
> review".
> Only one voucher, plus an l10n code review, is required for the
> localisation owner; no code review is required for the peers.
>
> Given that, as I understand it, it is technically complex to manage many
> CVS access restriction partitions, we have been giving people like
> localisers "full access" accounts and using social controls to prevent
> errant checkins. Will this approach continue?
>
.... just to the /l10n repository, which holds nothing but product
localizations.
I don't see a good pay-off for any kind of mapping of the localization
community structure onto a permissions system.
Basically, we're dealing with some 200 modules or more, one per locale
and toolkit, browser, mail, calendar. All of these have intermingled
ownership and peer releationships, with some parts of the source being
effectively owned by Mozilla, depending on branch, for productization.
Let's just ignore l10n in this discussion, and see if anything that
comes out of the discussion blends nicely with l10n.
Axel
Mitchell Baker
> Mitchell Baker wrote:
>> <snip>
>> Proposal
>>
>> 1. Number and Identity of Required Sponsors
>><snip>
>
> In the old policy, there was also the rule that code modules that did
> not require superreview did not require super-reviewers to get someone a
> CVS account. See the "Voucher and Super-reviewers" section on
> http://www.mozilla.org/hacking/getting-cvs-write-access.html .
> > What will happen to this? Will those people still need "just" two
> vouchers? Maybe keep it like that at least until discussion about the
> role/existence of super-reviewers is publicly dealt with?
>
> ~ Gijs
This is a really great question. It combines pretty well with
Nelson'original question; I'll try to respond to both here.
Nelson
I wasn't trying to do away with existing exceptions, sorry I wasn't
clear about this. It makes sense to look at exceptions and verify they
are working well and still make sense. Everything I've heard says that
the security modules are a classic example since they are stand-alone.
So while it probably makes sense to review our policy for all code, we
would want to not change those things that are working.
There's been some discussion above more fine-grained access controls; I
won't repeat that. I'm generally of the view expressed earlier that
allowing as much flexibility as possible, and setting up barriers only
when necessary is most effective. Others have made that point more
eloquently, so I'll stop there.
Gijs--
Looking at this document I think there will be some changes. Right now
the front-end of Fx is exempt from super-review, so I suppose one could
argue the source access rules don't apply. I don't think we want to end
up there. The goal of my proposal (and mconnor's efforts as well I
believe) is to have a CVS access policy that applies across the board
for firefox and thunderbird, and we look at other code to see if the
current setting is fine.
You're right; making changes for these project without having a
discussion with project participants would be a mistake. I agree with
your plan -- the things listed as "Exceptions" should proceed as they
have been until we've had a chance to evaluate. Nelson's chimed in the
security pieces; I'll ping the build folks as well.
mitchell
Stuart Parmenter
I feel pretty strongly that we don't want to make any exceptions for
the Mozilla project main development version control system and that
we should get rid of the ones we currently have. The Mozilla Project
should require a high bar for entry to help ensure that all Mozilla
projects continue to have high quality.
I don't see why our module owners or peers or super reviewers would be
unable to assess the level of worthiness of someone in NSS or anything
else currently with an exception. Code is code and the things they
would be looking for are not related to specific parts of NSS as much
as how the person interacts with the community, how their overall code
quality looks, etc. I believe it is worth that "3rd party's" time to
ensure that the people we are adding are providing quality
contributions to our codebase.
Even for those people who work in small areas of the code when they
start out, they often end up working on different areas later on or at
some point they produce some patch that touches code covered by many
modules. People shouldn't be able to play the system (intentionally
or not) by getting an account with a different set of requirements and
then moving in to areas that would have required a higher set of
standards.
stuart
Mitchell Baker
I understand you are suggestion we remove the exceptions for how one
gets CVS access (and it makes sense to me).
Are you suggesting we simultaneously get rid of the exception for the
areas of code that require super-review?
I think you're suggesting the former only (remove exceptions for CVS
access) but would like to be sure.
mitchell
Gijs Kruitbosch
>
>
> I feel pretty strongly that we don't want to make any exceptions for
> the Mozilla project main development version control system and that
> we should get rid of the ones we currently have. The Mozilla Project
> should require a high bar for entry to help ensure that all Mozilla
> projects continue to have high quality.
>
> I don't see why our module owners or peers or super reviewers would be
> unable to assess the level of worthiness of someone in NSS or anything
> else currently with an exception. Code is code and the things they
> would be looking for are not related to specific parts of NSS as much
> as how the person interacts with the community, how their overall code
> quality looks, etc. I believe it is worth that "3rd party's" time to
> ensure that the people we are adding are providing quality
> contributions to our codebase.
>
> Even for those people who work in small areas of the code when they
> start out, they often end up working on different areas later on or at
> some point they produce some patch that touches code covered by many
> modules. People shouldn't be able to play the system (intentionally
> or not) by getting an account with a different set of requirements and
> then moving in to areas that would have required a higher set of
> standards.
>
> stuart
>
Would you also want to re-evaluate the people who obtained CVS access in
such a manner, ie those who "intentionally or not" played the system
(such as myself, for instance?). I doubt you do, but I thought I'd ask.
I would also like to note that I don't *think* that there have ever been
problems with contributors who "played the system" in this manner doing
completely lousy commits. Even without superreviewers, you still have to
abide by the r/sr rules and the like. But I've by far not been around as
long as you guys, so perhaps I missed such fun.
~ Gijs
Stuart Parmenter
I don't think we need to go back and re-evaluate people who have CVS
access already. We've gotten lucky and have a great set of
contributors but moving forward I think we want to have a clear set of
requirements without exceptions for new contributors.
stuart
Robert Sayre
>
> I don't see why our module owners or peers or super reviewers would be
> unable to assess the level of worthiness of someone in NSS or anything
Fully disagree. NSS, NSPR, JS, and some other restricted areas govern
themselves effectively. I don't think there's anything broken, so I
don't see why we would want to encourage or dictate a process change.
There are other modules that don't restrict CVS access and don't require
superreview. I think it would be difficult to argue that any of these
meet the bar for self-governance.
- Rob
Reed Loden
Mitchell Baker <mitc...@mozilla.com> wrote:
> I understand you are suggestion we remove the exceptions for how one
> gets CVS access (and it makes sense to me).
>
> Are you suggesting we simultaneously get rid of the exception for the
> areas of code that require super-review?
Do you mean "don't require super-review"? Also, I hope not all the
exceptions are removed.
For example, webtools has only required a voucher for access instead
of the usual three SRs. With this new policy, would three vouchers or
two vouchers + one SR be required? It doesn't make sense to require an
SR to approve users that are webtools-only, as pretty much all the SRs
have nothing to do with the webtools at all.
Just wanting to make sure,
~reed
--
Reed Loden - <re...@reedloden.com>
Gervase Markham
> Let's just ignore l10n in this discussion, and see if anything that
> comes out of the discussion blends nicely with l10n.
Yes, apologies - I misunderstood the situation here. You are right - we
should leave l10n out of it.
Gerv
Gervase Markham
> We have not given localizers "full access" accounts on the main
> mozilla.org repository (with one exception a long time ago), as the L10n
> CVS is a different repository. Localizers only get (full) access to that
> separate repository.
Ah - I was not aware of that. Apologies.
I agree with your points as to the value of social as opposed to
technical controls. It's an example of decentralisation in
participation, to use a concept from the Mozilla Manifesto. :-)
Gerv
Gervase Markham
> I don't think we need to go back and re-evaluate people who have CVS
> access already. We've gotten lucky and have a great set of
> contributors but moving forward I think we want to have a clear set of
> requirements without exceptions for new contributors.
But do we have a great set of contributors because we've "gotten lucky",
or because the existing social controls system works fine?
Perhaps what's missing, at least from the current discussion, is the
rationale for any change at all. I went all the way back to mconnor's
message from May last year in mozilla.dev.planning, where he says "this
has come up several times" but I can't find a clear statement of the
problem we are trying to solve. (Perhaps I've missed it - pointers to
messages would be appreciated.) Is it:
- it's too hard/complicated/time-consuming to get CVS access
- it's too easy to get CVS access
- the wrong people are getting CVS access
- people get CVS access then do bad things
or some combination of the above?
(Note: asking for rationale is *not* me dogmatically saying "Why should
we change? I'm sure things are fine just now". Given that so many people
want it, there probably are good reasons for change. I just think that
articulating the problem clearly can help us find the right solution.)
Gerv
Mike Connor
On 12-Mar-07, at 11:21 PM, Reed Loden wrote:
> On Mon, 12 Mar 2007 17:08:40 -0700
> Mitchell Baker <mitc...@mozilla.com> wrote:
>
>> I understand you are suggestion we remove the exceptions for how one
>> gets CVS access (and it makes sense to me).
>>
>> Are you suggesting we simultaneously get rid of the exception for the
>> areas of code that require super-review?
>
> Do you mean "don't require super-review"? Also, I hope not all the
> exceptions are removed.
>
> For example, webtools has only required a voucher for access instead
> of the usual three SRs. With this new policy, would three vouchers or
> two vouchers + one SR be required? It doesn't make sense to require an
> SR to approve users that are webtools-only, as pretty much all the SRs
> have nothing to do with the webtools at all.
The point of the SR requirement is to act as a check against group
optimism, as Mitchell pointed out. I have no doubt that nearly all
of the SRs can look at the block of checkins from an individual and
evaluate them appropriately.
-- Mike
Robert Kaiser
> - it's too hard/complicated/time-consuming to get CVS access
From my experience in my areas, it's this one usually. In times where
active super-reviewers are getting less just because sr isn't needed any
more in many areas, and in times of more modularized development, it's
often hard to get three srs to review someone applying.
There is a requirement that every one of those srs knows the code of the
person applying for the CVS access, but many people have only
contributed to areas that don't need sr - and therefore (almost) no sr
has actively read that person's code yet. And if an sr knows the code,
that's usually the one in the code area the person is mostly working in.
An example:
In the case of the SeaMonkey project, we currently have two people I
know of that are applying for an account (Stefan Hermes, bug 357831 and
Frank Wein, bug 362633). Both are contributing a lot (mostly smaller
patches, some bigger stuff) and have done so over a longer time, but it
looks like it's hard to get more than one sr fro them, as only people
from within the SeaMonkey project dare to sr that - others just have
never needed to sr their code and don't know the code themselves. And
finding a sr who familiarizes himself with a bunch of different patches
from such a contributor to sr that cvs account request is quite hard.
Finding vouchers, even a bigger number, is much easier, as there are
many people knowing their code, who did reviews, checkins, etc.
I think there's the real problem we should try to solve here, while
still leaving the bar high enough to only get people in who fit into
this meritocracy of our repository.
Robert Kaiser
Boris Zbarsky
> - it's too hard/complicated/time-consuming to get CVS access
In some cases, yes.
> - it's too easy to get CVS access
In some cases, yes.
> - the wrong people are getting CVS access
Some think so, yes.
-Boris
Stuart Parmenter
> For example, webtools has only required a voucher for access instead
> of the usual three SRs. With this new policy, would three vouchers or
> two vouchers + one SR be required? It doesn't make sense to require an
> SR to approve users that are webtools-only, as pretty much all the SRs
> have nothing to do with the webtools at all.
>
I am suggesting we remove this exception as well for basically the
exact reason mconnor says below:
On Mar 13, 8:22 am, Mike Connor <mcon...@mozilla.com> wrote:
> The point of the SR requirement is to act as a check against group
> optimism, as Mitchell pointed out. I have no doubt that nearly all
> of the SRs can look at the block of checkins from an individual and
> evaluate them appropriately.
>
Yes, exactly.
Stuart Parmenter
> Stuart
>
> I understand you are suggestion we remove the exceptions for how one
> gets CVS access (and it makes sense to me).
>
> Are you suggesting we simultaneously get rid of the exception for the
> areas of code that require super-review?
>
> I think you're suggesting the former only (remove exceptions for CVS
> access) but would like to be sure.
>
> mitchell
>
I'm not trying to suggest any SR related changes for code review
here. I'm suggesting "no exceptions for CVS access, everyone must
comply."
stuart
Gervase Markham
> Gervase Markham wrote:
>> - it's too hard/complicated/time-consuming to get CVS access
>
> In some cases, yes.
>
>> - it's too easy to get CVS access
>
> In some cases, yes.
I've seen examples of the former from kairo; do you have an example of
the latter? I guess it's tied in with the following:
>> - the wrong people are getting CVS access
>
> Some think so, yes.
While it's not nice to point the finger at particular people, it's hard
to see how much of a problem this is without examples. Are there tree
bustages, or failures to follow protocol that we can point to to say
that "this person should never have got CVS access" or "this person got
it before they were ready for it"?
Gerv
Mitchell Baker
Well, I'm not sure this is really going to be helpful. We have a
system that hasn't been reviewed or updated for years, we have new
organizational roles that aren't reflected; right now the toolkit module
doesn't need SR so technically maybe there isn't a CVS policy for it,
and that's not good.
So I hope we can talk about good policy without going backward to look
at people.
mitchell
Gervase Markham
> So I hope we can talk about good policy without going backward to look
> at people.
OK. I guess the point I was getting at is that if we think bits of the
policy are broken, we should fix those bits, but if bits are working, we
should not change them in the name of 'a foolish consistency'.
The CVS access requirements for Webtools, NSS, JSS and NSPR are
acceptable to the leaders of those parts of the community. Unless
someone from another area has an example of something bad which has gone
wrong in their area as a direct result of those policies, we should
leave them as they are.
But having re-reviewed the entire thread, I think you agree with this
anyway (because you said so to Nelson).
Gerv
Mitchell Baker
Gervase Markham wrote:
> Mitchell Baker wrote:
>> So I hope we can talk about good policy without going backward to look
>> at people.
>
> OK. I guess the point I was getting at is that if we think bits of the
> policy are broken, we should fix those bits, but if bits are working, we
> should not change them in the name of 'a foolish consistency'.
>
> The CVS access requirements for Webtools, NSS, JSS and NSPR are
> acceptable to the leaders of those parts of the community. Unless
> someone from another area has an example of something bad which has gone
> wrong in their area as a direct result of those policies, we should
> leave them as they are.
Well, the CVS access privileges for the tree are not privileges of just
a small group. So I'm not so sure that keeping that small group happy
is the only criteria. It may be that the policies for these groups are
just fine (although as I say I'm coming to see Stuart's point.)
>
> But having re-reviewed the entire thread, I think you agree with this
> anyway (because you said so to Nelson).
>
> Gerv
mitchell
Gervase Markham
> Well, the CVS access privileges for the tree are not privileges of just
> a small group. So I'm not so sure that keeping that small group happy
> is the only criteria.
I agree it's not the only criteria. But I don't see what would be
achieved by changing the requirements for NSS, JSS, NSPR or Webtools.
No-one has yet stated an actual problem that doing so would solve.
The problems for the main part of the tree, on the other hand, have been
documented and there is a clear case for review and change.
Gerv
Gervase Markham
> Comments welcome. If I don't hear significant concerns, I'll turn to
> getting the official documents on the website updated to reflect the
> content below.
The question of the existing exceptions to the process (NSPR, NSS/JSS,
Bugzilla and other webtools) was raised somewhere down this thread. I
emailed the maintainers of these projects to ask them what their
procedures were, so we could see how different (or not) they were from
what is being suggested.
These are my summaries of their emails; any mistakes are my own.
NSS (Wan-Teh Chang)
-------------------
The people who get CVS write access to NSS fit into one of two
categories. In both cases, the CVS account application needs to be
vouched by one NSS/JSS module owner in place of a "super reviewer".
1. People who demonstrate knowledge of NSS and/or sufficient knowledge
of crypto (including knowledge of security principles for crypto use),
and who contribute substantial amounts of code to NSS that fixes real
problems or implements useful features. They are usually, but not
always, employees of the companies that fund NSS/JSS development. The
companies changed over time but have always been reputable companies
(Netscape, AOL, Red Hat, and Sun). Douglas Stebila is one known exception:
https://bugzilla.mozilla.org/show_bug.cgi?id=329235
2. People who support the core NSS team, whose jobs require them to have
write access to NSS, such as build and QA engineers and junior
developers, who may not fit the above criteria but who nonetheless
perform real and useful functions in NSS development and are under the
tutelage of NSS core developers. These people's access to NSS is tied
to their employment, and ends if/when they change jobs, unless during
their employment they move into the other category.
At one point, the project leader of a large code contribution considered
applying for a Mozilla CVS account, but thought the process too complex,
so the contribution was committed on his behalf by an existing committer.
NSPR (Wan-Teh Chang)
--------------------
The NSPR process is built on top of either the Mozilla or NSS processes.
Someone with CVS access via one of those processes who shows competence
in NSPR development is given access to check in to the NSPR partition.
Bugzilla (Dave Miller)
----------------------
"The requirements of Bugzilla are basically that we offer someone CVS
access when they are producing enough good patches that those who
already have CVS access tire of committing patches on their behalf.
There's not really a cut and dried quantity or anything. They do still
have to have senior project members vouch on their behalf."
Gerv
Mitchell Baker
The only open question that I know of is how to define exactly what code
is covered by the policy. The original rule is: if code needs
super-review then the CVS write access rules apply.
And code that needs super-review is defined as: "code that is
"in-process" or built with the Mozilla browser/mail/news/editor
application suite . . . "
So non-client code (bugzilla, etc) are not included in the current CVS
access requirements. In addition, there is some code in the client
products that are specifically excepted – NSPR and NSS for example.
Both these criteria need updating. The suite is no longer our shipping
product. And significant parts of the shipping product aren’t using the
super-review system currently.
So I’d propose two steps:
First step, to be taken now: use the new policy for any code in modules
used by Firefox, Thunderbird or the Seamonkey suite. BUT, leave the
current exceptions in place: NSPR, NSS, etc.
This improves the setting for a vast number of people.
Second step, address the exceptions. For example, I think the calendar
project should be subject to the same CVS access rules. But I’d like to
talk with them before making this a requirement. Stuart suggests all
code, including webtools should be included in these rules. I’d like to
explore this, I think I agree in principle but we haven’t had a real
discussion about this yet. I’ll open a separate bug on this.
I’m working now on updating the posted policy document to reflect this.
Incremental improvement is a fine technique.
If anyone has objections please let me know.
Mitchell
Mitchell Baker wrote:
> Here's a proposal. It's based on the previous thread, and on a bit of
> poking around I did to assure myself thatthe role of super-review and
> the identity of super-reviewers is likely to continue. (Bug 366103 is
> the tracking bug for the role of super-review. We'll have that
> discussion publicly soon. But for now I'm confident enough that we
> won't eliminate super-reviewers to keep them in the proposal below.)
>
> Most significant changes from mconnor's suggestions are requiring one
> SR, and the SR to be from somewhere else in the codebase; and adding
> something explicit about code quality. Thanks to mconnor for doing most
> of the hard work.
>
> Comments welcome. If I don't hear significant concerns, I'll turn to
> getting the official documents on the website updated to reflect the
> content below.
>
> ml
>
>
>
> Proposal
>
> 1. Number and Identity of Required Sponsors
>
> -- One needs three sponsors: two vouchers and a super-reviewer.
>
> -- Each of the two vouchers must be a module owners or peer. They can
> be from the same module, and they should be from the module(s) where the
> proposed committer committed patches.
>
> -- The third person must be a super-reviewer, and shuldn't be from the
> same modules as the two vouchers. This breath requirement is intended
> as a check on group optimism.
>
> (The old policy required 1 voucher and 2 or 3 super-reviewers).
>
>
> 2. Role of Vouchers
>
> -- Vouching is a commitment to work with the new committer until he or
> she is ready to work indpendantly. This means the vouchers should keep
> tabs on check-ins from the new committer and help resolve issues that
> might arise, such as awareness of closed trees, untended regressions,
> prompt response to bustage, etc.)
>
> -- If some Vouchers are consistently over-optimistic about the people
> they vouch for we'll figure out what to do. I'd rather not set up an
> elaborate policy about it's clear we need.
>
>
> 3. Criteria for Access
>
>
> --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
>
> 4. Employement of Sponsors
> At least one of the three sponsors must not have the same employer as
> the proposed committer.
>
>
> mitchell