Proposing changes to granting CVS access, Round 2
Mike Connor
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
Robert Accettura
if there's no clear agreement.
I'd rather put more emphasis on patches committed rather than time period. I'd
say someone who has submitted 10 patches in 3 months is more experienced than
someone who did 3 in 10 months.
Regardless, if a time period is a requirement, shouldn't it be defined? One
product release, 3 months, etc. etc. Whatever is agreed upon.
Just my $0.02
--
Robert Accettura
rob...@accettura.com
Quoting Mike Connor <mco...@mozilla.com>:
> _______________________________________________
> dev-planning mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-planning
>
Mike Connor
On 20-Nov-06, at 3:16 PM, Robert Accettura wrote:
> I'd rather put more emphasis on patches committed rather than time
> period. I'd
> say someone who has submitted 10 patches in 3 months is more
> experienced than
> someone who did 3 in 10 months.
Trying to make this a hard and fast rule is doomed to failure.
Consider graydon's cycle collector, etc. Its definitely something we
view as subjective as a group, based on the discussions I've had on
the subject. This would just create a system to game (do more small
patches, rather than larger changes), which would cause unnecessary
overhead.
> Regardless, if a time period is a requirement, shouldn't it be
> defined? One
> product release, 3 months, etc. etc. Whatever is agreed upon.
Its a subjective guideline for a reason. if someone is obviously a
solid contributor after a month, why would we have a hard line there?
-- Mike
Gijs Kruitbosch ("Hannibal")
What about being on hook? I mean, the emails bonsai sends, as well as
some of our documentation on the matter, are dated (unless I've missed
something in the year or so I've had cvs access now) in the sense that
they claim you must be available at the next nightly pull + build -
you're simply expected to watch the tree and make sure your changes
don't break it. AIUI. Are those explanations (ever) going to be fixed,
or do we want to keep it that way for nostalgia's sake, or something? :-)
Apart from that, I like these requirements, and I'd love it if they
could replace the vague ones we currently have up on www.mozilla.org.
-- Gijs
Boris Zbarsky
> What about being on hook? I mean, the emails bonsai sends, as well as
> some of our documentation on the matter, are dated (unless I've missed
> something in the year or so I've had cvs access now) in the sense that
> they claim you must be available at the next nightly pull + build -
> you're simply expected to watch the tree and make sure your changes
> don't break it.
Actually, last I checked you're expected to do three things:
1) Watch the tree and make sure you don't break it or seriously regress any
tests or anything else that would require tree closure. This watching
covers the time until all tinderboxen (including ports, etc) cycle with your
change.
2) During this time you must be available (irc preferred, but IM acceptable if
the IRC bots know about it; e-mail NOT acceptable due to higher latency)
to answer sheriff questions about problems arising from your checkins. If
there's a problem and you're not around, you should be backed out, since
there's no way to tell when you'll get around to fixing it or whether you're
even watching the tree.
3) Be around the next day and keep an eye on the blocker bugs filed that day
on the nightlies with your changes; if one of those blockers is due to your
patch you drop everything and fix it or back out until you have time to fix
it.
Nowadays people are generally on top of #1. A number of people don't do #2 or
#3, though. This last is sort of working out due to the very small number of
blocker bugs that arise, compared to days of yore. But #2 has been a serious
issue in the recent past.
Note that we should spell out the hook responsibilities for people when we give
them CVS accounts. Right now we don't, which is a problem. I suspect most
people have no idea that they're supposed to not only watch the tree but be
responsive to sheriff queries while they're on hook.
I do agree that the concept of "on hook" as "until the next build has been
smoketested" is meaningless if we're not doing scheduled daily smoketests.
-Boris
mconnor
We should change/drop the hook email, definitely, and clarify around responsibility for regressions etc (the main reason for the hook's nightly build req). That's something we should just file a bug on and maybe figure out how to make useful.
- Mike
[Message delivered by NotifyLink]
----------Original Message----------
What about being on hook? I mean, the emails bonsai sends, as well as
some of our documentation on the matter, are dated (unless I've missed
something in the year or so I've had cvs access now) in the sense that
they claim you must be available at the next nightly pull + build -
you're simply expected to watch the tree and make sure your changes
don't break it. AIUI. Are those explanations (ever) going to be fixed,
or do we want to keep it that way for nostalgia's sake, or something? :-)
Apart from that, I like these requirements, and I'd love it if they
could replace the vague ones we currently have up on www.mozilla.org.
-- Gijs
Gijs Kruitbosch ("Hannibal")
Ah, well, for my own conscience, I do comply with #2, and I watch the
default QA on bugzilla for the module 99% of my changes are in
(ChatZilla, so not like that affects anyone else), so I'll know if
blockers get filed. Good points though :-)
-- Gijs
Robert Kaiser
> Thoughts?
This all sounds very good...
Does that mean someone who has done work in only one module (say,
SeaMonkey, i.e. suite/) can get cvs access if three owner/peers of that
module vouch for him?
In particular, I'm wondering if this new process would help us to push
forward requests such as the one of stefanh -
https://bugzilla.mozilla.org/show_bug.cgi?id=357831
Robert Kaiser
Mike Connor
On 20-Nov-06, at 9:18 PM, Robert Kaiser wrote:
> Mike Connor schrieb:
>> Thoughts?
>
> This all sounds very good...
>
> Does that mean someone who has done work in only one module (say,
> SeaMonkey, i.e. suite/) can get cvs access if three owner/peers of
> that module vouch for him?
Yes, exactly.
> In particular, I'm wondering if this new process would help us to
> push forward requests such as the one of stefanh - https://
> bugzilla.mozilla.org/show_bug.cgi?id=357831
That's exactly the type of case I'm hoping to deal with better with
this set of changes.
-- Mike
Frank Wein
> To see the prior history on this, check out http://groups.google.com/
> group/mozilla.dev.planning/browse_thread/thread/f1636813ca66e35c/
> 888f829c6f368205?
Better link (and more permanent since it includes message id ;):
http://www.google.com/groups?threadm=mailman.2094.11485110...@lists.mozilla.org
Frank
Axel Hecht
I'd like to make up a corresponding scheme for l10n. I drafted that at
http://wiki.mozilla.org/L10n:Ownership, which hasn't gotten a whole lot
of feedback so far from the l10n community, sadly.
Whichever, the language chosen here should probably point elsewhere and
we probably have to keep our knees bent for any impact of the changes
that a switch in RCS might bring.
On the suggested points themselves:
I'm not so sure about the three vouchers. Wouldn't three reviewers take
away from the responsibility each voucher takes?
On the time period, should we really keep folks like new hires not
coming from the community out of checking in (and fixing their
bustages)? I wonder if those folks (at least those in the office) should
have something like "two vouchers, who each interviewed the candidate,
and a period where the candidate doesn't check in without someone
holding his/her hand". Basically, a mentor.
Axel
Mike Connor
On 21-Nov-06, at 3:37 PM, Axel Hecht wrote:
> I'm not so sure about the three vouchers. Wouldn't three reviewers
> take away from the responsibility each voucher takes?
I think that the traditional single voucher oversight aspect isn't
well-used these days, and isn't strictly necessary.
> On the time period, should we really keep folks like new hires not
> coming from the community out of checking in (and fixing their
> bustages)? I wonder if those folks (at least those in the office)
> should have something like "two vouchers, who each interviewed the
> candidate, and a period where the candidate doesn't check in
> without someone holding his/her hand". Basically, a mentor.
I don't think employer should make any difference in how you obtain
CVS access. If we bring someone in from outside, they should earn
the trust and respect of the community, in the same way as a fulltime
contributor. This is actually something we try to make clear to
interviewees and new hires.
Simon Paquet
> 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)
So that would mean, that a single voucher by the module owner in
strongly-owned modules (like Calendar) will no longer be sufficient?
This was the case for me (see bug 355394).
Mike Connor
On 22-Nov-06, at 8:04 AM, Simon Paquet wrote:
>
> So that would mean, that a single voucher by the module owner in
> strongly-owned modules (like Calendar) will no longer be sufficient?
>
> This was the case for me (see bug 355394).
The problem we've encountered in applying that rule to browser/
toolkit is that we're in effect saying "you can only check into those
directories" and patches only stay constrained in those areas for so
long. What happens when you end up with a patch that fixes a toolkit
bug as part of a larger fix? Most people just end up landing
anyway. I have avoided using that exemption for browser/toolkit
hackers for that exact reason, and I don't think its especially
useful for things other than parts of the codebase that are
standalone building blocks like SpiderMonkey and NSS. I wasn't
around for its inception, but I think that was a compromise for the
three SRs rule, since its hard to get SRs to vouch if they'll never
SR your patches.
-- Mike
Robert O'Callahan
> * At least one voucher should have a different employer than the
> requestee (goes to community involvement more than anything else)
This is going to get harder and harder as Mozilla.com grows and employs
a larger and larger percentage of the community. (Not that I'm saying
that's a bad thing!)
How about this idea ... if you already have commit access for another
high-profile open source project, we allow that to count as a "different
employer" voucher.
Rob
Mike Connor
On 22-Nov-06, at 9:47 PM, Robert O'Callahan wrote:
> Mike Connor wrote:
>> * At least one voucher should have a different employer than the
>> requestee (goes to community involvement more than anything else)
>
> This is going to get harder and harder as Mozilla.com grows and
> employs
> a larger and larger percentage of the community. (Not that I'm saying
> that's a bad thing!)
Personally, I want to grow the community in proportion, but that'll
be cyclical in the best case. :)
> How about this idea ... if you already have commit access for another
> high-profile open source project, we allow that to count as a
> "different
> employer" voucher.
I'm down with that, though some projects (i.e. KDE) seem to have a
pretty low bar there. Obviously we'd need to have a vetted list in
place of projects that we consider to have appropriately high bars
for commit access. Its a good way of not treating people like
they're totally new, as long as the remaining vouchers ensure the
requirements are met.
-- Mike
Robert O'Callahan
> On 22-Nov-06, at 9:47 PM, Robert O'Callahan wrote:
>> How about this idea ... if you already have commit access for another
>> high-profile open source project, we allow that to count as a "different
>> employer" voucher.
>
> I'm down with that, though some projects (i.e. KDE) seem to have a
> pretty low bar there. Obviously we'd need to have a vetted list in
> place of projects that we consider to have appropriately high bars for
> commit access.
Yes.
Rob
Christian Biesinger
> * Has enough of a track record to indicate the above (at least a couple
> of months of active hacking)
Your proposal generally sounds good to me, but I think that requiring a
couple of months is too long... IMO, <= 2 months should be sufficient in
most cases, and I wouldn't call that "a couple".
--
All the world's a stage,
And all the men and women merely players:
They have their exits and their entrances;
And one man in his time plays many parts, [...] --W. Shakespeare
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
Mitchell Baker
Proposed Changes
1. Required Approvals
2. Role of Vouchers
3. Breath of Vouchers
mitchell
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
Aaron Leventhal
The new policies would help the a11y module quite
a bit. For example, I'd like to get Alexander
Surkov CVS access. He's been a great coder for the
mozilla/accessible module. We haven't been
requiring sr='s for many changes. The current
process is very cumbersome.
- Aaron
Mike Connor wrote:
> Sorry for the delay here, this ball fell on the floor.
>
> 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