Many teams have some sort of very regular reviews. I’m not thinking personnel reviews or budget reviews, I’m thinking code reviews specifically but it could be test reviews, documentation reviews or some other. Reviews that need to happen every day but which frequently get delayed.
Lets stick with code reviews because they are the type I encounter most often.
Code reviews are good, by some accounts they the most effective means of removing bugs early – although I haven’t seen code reviews compared with TDD. But, code reviews loose their efficacy when they are not conducted promptly. The longer the period between the review being requested and the review being conducted (and by extension the review comments being acted on) the less effective the review.
The effect reduces because: a) the person who requested the review has moved onto something else, b) issues found in the review may be repeated until the review is conducted and c) the review will either inject a delay into the delivery process or the delivery will happen without the review in which case, what was the point?
So: if you are going to conduct reviews you want them to happen soon.
And it is not just the code and the developer who wrote it who have problems. The designated reviewer feels the pressure to “do reviews” when they have other – important! – work to do.
One team I know came up with a simple solution to this problem. I recently recommended the solution to another team who promptly adopted it and are delighted. One developer said: “I’ve never been so relaxed about reviews before.”
The solution is…
Make reviews the first item of work after the stand-up meeting in the morning. And let people do them before the stand-up too.
Thus, as soon as the stand-up is finished everyone undertakes any reviews which are needed before they start their day’s work. Review work is prioritised before new work: after all, yesterday the thing that now requires review was the priority, it is probably still the overall priority and the only thing standing between it and “done” is a review.
Reviews don’t typically take very long so today’s work isn’t delayed. And the recipient of the reviews comments can act on the comments before they get into today’s work.
Better still, knowing that reviews will happen right after the stand-up meeting means that it also makes sense to do the reviews BEFORE the stand-up meeting. This also addresses the question of “how do I usefully use the time before the stand-up meeting when I know I’ll have to stop doing whatever I start.”
So for example, image on Tuesday afternoon Bob asks Alice to review the code he has just finished. If Alice is available she might just do it there and then. But if she is busy she will wait. Now if she arrives at work at 9am and the stand-up is 9.30am she can get on with the review before the stand-up, if she finishes Bob will have his feedback and can act on it before 10am. If Alice doesn’t get to Bob’s code then she will do it at 9.45am when the meeting finishes.
Either way, Bob isn’t left waiting.
In part this works simply because it keeps the review queue short. If reviews are done soon, say within 24 hours, then the queue is never allowed to become too big and thus its easy to keep the queue short.
One of the teams actually put a column on their board where tasks awaiting review could rest. In the stand-up everyone could see what was waiting for review and arrange to do it.
Simple really.
While we are on the subject of code reviews let me comment on something else.
There is often a belief that only senior developers, or only “architects” should conduct reviews. I think this approach is mistaken for two reasons.
Firstly in this model it is normal that there are far fewer reviewers than there are review requesters. This frequently results in queues for reviews because the pool of reviewers – who also have other work to do – is small.
Second this model assumes that only “architects” can make useful comments on code. I believe most, say 80%, of the efficacy of a code review is the result not of having an expert review the code but simply the result of having another person review the code.
Indeed I even go as far as to say junior people should review senior people’s code. Code reviews are a two way learning process – one of the reasons I like to see them done face-to-face. If an experienced developer is writing code that a junior cannot understand (why do I think of C++ meta-templates?) then the experienced person should know that they are writing code other people cannot maintain.
Anyway, there you go, let me know if you try this idea and what the result is.
| Reference: | Code and other reviews (a small piece of advice) from our JCG partner Allan Kelly at the Agile, Lean, Patterns blog. |
