On 11/6/12 10:09 AM, Dave Townsend wrote:
We've had a policy requiring super-review for certain kinds of patches
for a long time. It's changed a couple of times but the current policy
(http://www.mozilla.org/hacking/reviewers.html) primarily requires
super-review for any patch that introduces or changes an API.
I think we should consider jettisoning/rewriting that part of the
policy. It doesn't match what we've been doing in reality(*), and we
don't seem to be in a terrible state as a result.
A bit of data: by my crude Bugzilla search, I count 104 sr+'d bugs that
have been fixed in the last 3 months. (http://is.gd/45WAxl) By product:
Core - 75
Toolkit - 9
Mailnews - 8
NSS+NSPR - 6
TB+SM+CM - 6
(*) Core's pretty clearly still using sr+, so if it's working and they
want to keep it that's just fine. I direct my comments at Toolkit and
Firefox.
The policy page goes into the motivations a bit, which are generally
sound, but I think that a big part of the problem is that it's presented
in absolutest terms... Any API change. Any cross-module change. Only
refactoring has some wiggle room via "significant". These things all
vary in degree as well as the cost for making a mistake.
I think we should consider rewriting these rules as guidelines and
placing the expectation on reviewers to determine if/when superreview is
needed. This isn't that different from how reviewers operate in Toolkit
and Firefox now... Instead of hyperspecialized reviewers, it's a general
pool with the expectation that members are able to self-determine what
they're able to review or when they should flag someone else for
additional review.
This would also help from a new-contributor angle -- the whole "what is
sr? When do I need it? Who does it?" question is hard to answer until
one learns the ropes. But it's reasonable for reviewers to know.
Justin
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
