14791 - Need better checkin review/approval rules for M11 and beyond

Status: VERIFIED FIXED

(mozilla.org :: Miscellaneous, task, P3)

Description

[Brendan Eich [:brendan]]

Until we get to dogfood, we need a scalable system for approving checkins.  It
can't be one person, so we need to keep the sheriff rotation going.  The sheriff
may approve a checkin if:

1.  It fixes a bug in chofmann's 14468 beta-key dependency tree (if it fixes a
truly must-fix for beta bug that's not in the 14468 tree, the sheriff needs to
get brendan or chofmann to update that tree).

2.  The bug number must be cited in the checkin message, but it should not be
the only content in the log message (i.e., say what you fixed, and put the bug #
and a= r= lines in parentheses).

3.  The fix has at least one code reviewer.

4.  The reviewer is cited in the checkin message (bug #, r=[e-mail],
a=sheriff).

5.  If the sheriff is temporarily unavailable or swamped, brendan or chofmann
can approve and update 14468 as necessary.  Again, we're trying to avoid single
points of failure.

/be

Comment 1

[brendan-obsolete]

Update: 14468, the beta-key bug, is not the right bug because it would keep too
many people idle.  We need another meta-bug, or target-milestone, or equally
well-marked way for sheriffs to see so they can approve checkins.  This checkin
approval process needs to keep going after the milestone shutdown, until
December if not beyond.

/be

Comment 2

[Peter Trudelle]

Is there still a sheriff rotation?  I got the day off today, courtesy of
chofmann.  I thought it would be too much for any one person too, but I
think that whoever is approving should also be acting as sheriff.

Comment 3

[chris hofmann]

I think we have all the review/approval rules we ever wanted and more...

Comment 4

[Chris McAfee]

We should at least post the current rules somewhere,
what's the current URL?  "Top of tinderbox" is too fluid and
doesn't count.

Comment 5

[Chris McAfee]

reopening.

Comment 6

[Brendan Eich [:brendan]]

Dawn, can you update some near-top-level doc to give the rules?  Or should your
docs just link to a projects/seamonkey doc?  If so, bounce this to jar, cuz he
should be maintaining those docs.

/be

Comment 7

[leger]

Clearing FIXED resolution due to reopen of this bug.

Comment 8

[chris hofmann]

cleaning up m12 bug list to only critical items.

Comment 9

[chris hofmann]

top of tinderbox is the one place we can get everyone to look.
its a good sole source for what to do before durning and after checkin.

what do we need to do to close this? ideas?

Comment 10

[Chris McAfee]

Having n number of reviewers seems to be working,
as long as n > 2 and some irc presence is there.
Marking fixed.

Comment 11

[Peter Trudelle]

I'm suspicious of the 2 reviewer requirement.  I think that at best it provides
only marginal improvement over having one good reviewer (with at least twice the
cost), and could reduce individual responsibility for the checkin (the 'Kitty
Genovese' syndrome).

Comment 12

[Chris McAfee]

reopening, any better ideas?

Comment 13

[leger]

Clearing FIXED resolution due to reopen.

Comment 14

[Dawn Endico]

removing milestone

Comment 15

[Chris Yeh]

mass re-assign of all bugs where i was listed as the qa contact

Comment 16

[Mitchell Baker]

Brendan, Dawn

shouldn't this either be closed, given current text on tinderbox, or reassigned 
to Brendan to figure out new rules?

Comment 17

[Asa Dotzler [:asa]]

I think this can be resolved FIXED and any remaining problems filed as new bugs
against the current system.  Changing product and component, setting Resolution
to FIXED.

Comment 18

[timeless]

rs verifying post m11 new rules went into place. any future needs can be
addressed in future bugs.