Platonides Platonides@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.