Platonides <Platonides(a)gmail.com> writes:
And no, nobody wants our review paradigm to be
"let's spend several
months on the backlog every time we want to release". It was just the
best we managed to afford.
We've been doing a little better for the past month, but Robla's
chart[1] is still looking ugly.
So, other than switching to the mythical GIT, where all is rainbows and
roses, what can we do to improve code review now?
As far as I can see, the main reason that people think code review
works better under GIT is because the committer is responsible for
getting xyr[2] code reviewed *before* it is merged. The committer is
motivated to find get xyr code reviewed because if xe doesn't, the code
won't be used, and others will not experience its beauty.
Subversion doesn't support this review-first model. Not unless we set
up another branch that only took reviewed code. But then, we're back in
the same boat. Most people would run from trunk and the committer knows
that xyrs code will be used by a lot of people and extensions will
probably be developed that depend upon it, etc.
So, while Subversion doesn't support review-first, it can incorporate
revert-later. We can even use our current CodeReview tool. We just
need to be more aggressive reverting unreviewed code.
And just to be clear, there would be a not-too-distant “later”. I
propose a week.
If code is to survive past a week in the repository, it has to be
reviewed.
If you want to make a commit that depends on un-reviewed code, you have
to find someone to review it. Otherwise, your commit will break trunk
when that code is reverted.
FIXMEs would disappear. FIXMEs would be up for reversion almost
immediately. Give the committer a day to fix the code, but if it
survives 24 hours as a FIXME, it gets reverted.
I suggest we implement this ASAP. If we start this policy on April 4th,
we would be doing the first round of reverts April 11th. We should
grandfather in the current code, of course. It would be exempt from
grim reversion reaper, but it should still be reviewed.
This solution would mean pain, but I think it would be manageable pain.
And it would be more workable than the changing the vcs that the twn
people have to work with.
Thoughts?
Mark.
Footnotes:
[1]
http://toolserver.org/~robla/crstats/crstats.html — the problem is
easiest to see if you unclick “ok”. Then you'll see the red “new”
line is creeping up again.
[2]
http://en.wikipedia.org/wiki/Gender-neutral_pronoun, equivalent to
“his or her” but only jarring (till you get used to it) and not
cumbersome.