On 04 Jun 2010, at 2:55 AM, William A. Rowe Jr. wrote:
If there is not positive feedback from two reviewers, this code
does not
belong in trunk/. As a committer, you are *free* to create your own
sandboxes in svn to demonstrate code changes, if that helps
attract the
necessary review.
What you're describing here is review-then-commit (RTC).
No, I wasn't. What I was suggesting is that code that is missing
the 'then
Commit' bits of RTC doesn't belong. It is not reassuring when
committers
aren't reviewing the patches offered when they are presented.
Committing
them to trunk doesn't reassure me that they'll have sufficient
review after
the commit, either.
I get the strong sense that you want to "patch over" problems like
this by adding additional layers of bureaucracy, instead of fixing the
underlying problem - a shortage of active committers.
The ASF trusts committers. Committers are trusted not only to produce
good code, but also to look over the shoulders of other committers and
review code. This you-watch-my-back-and-I-watch-yours constitutes a
very strong level of redundancy in our development process - a problem
introduced is very unlikely to remain undetected, and this is evident
in the excellent stability of our codebase.
This however only works if the level of active committers is kept up.
Naturally over time the level of contribution of committers will
change as priorities change, personal interests change, etc, and this
causes a natural erosion of the active committer level. This needs to
be balanced with the introduction of new committers, before a new new
committer feels their contribution is being ignored and diverts their
energy elsewhere.
Adding the additional bureaucracy as you propose will only make the
problem worse. Instead of it taking a long time for code contributions
to be accepted like now, code will be actively rejected due simply to
disinterest by a too-small group of existing committers, and not
because of the quality of the code.
CTR is fine for all normal fixes.
No, it has been the policy of the project for many years that CTR is
fine on trunk for *all* contributions.
We have historically added the "if it's big, check first" proviso as a
sanity check to make sure nobody goes off on a big tangent, but lately
this has turned into just "check first", and "check first" means
"review".
RTC is always preferred for major code
refactorings. But the reviews need to happen, and this particular
code has
been available for discussion at dev@ for months, with very little
comment
between very few committers, which isn't a healthy sign.
I 100% agree it is not a healthy sign, but we disagree on the the
sign. I see it as a sign we must step up and propose some new
committers, I don't see it as a sign of a lack of code quality.
Regards,
Graham
--
