This post is over 6 months old. Some details, especially technical, may have changed.

Crucible Code Review Guidelines

I've been using Crucible on a number of projects for the last few months and I love it.  So now comes the time we want to try and make a case for it to be rolled out to other projects in the company I work for.  Along with the usual stuff such as cost of licenses, logistics, hardware need and so on I also need to consider education and best practise guidelines (oh I said “best practise” that term makes me feel slightly queasy) because after all — if people aren't using it right they probably wont see the benefit, stop using it and then we are back to square one with wasted time, money and effort.

Some of you may be thinking,

“But it's just a code review tool - what guidelines do you really need?”

But truth be told you do need some, and I learnt this the hard(ish) way.  So here are some top tips to getting the most out of Crucible and at the same time possibly embettering (not a real word but should be) your development approach.

Rule 1: Review Early, Review Often

Don't wait for a whole week before committing 600 files as a single review — no ones going to actually do a proper review as they don't have time or patience.  Generally speaking your work should be sufficiently broken down so that you can do a few hours of work, commit that code and submit that changeset as a review.  Typically speaking a small review may only take about 5 minutes or so to review per person which is a lot easier than having to actually book time to read an entire books worth of code.  Obviously don't be going insane on this.  A code review should encompass a complete feature and not some arbitrary files that are only a small piece of the puzzle.  Feel free to merge changesets where necessary too.

Rule 2: Refrain from adding new content to old reviews

I've seen a few cases were a bunch of files have been added to a review and this review has been used over and over with people adding files and new versions until the whole thing had pretty much the entire codebase included.  Needless to say that's wrong.  Create a review and don't be adding new files or versions too it unless it's in the early stages of review and it is deemed necessary.  Don't be afraid of kicking off more than one review - it's not a crime, in fact it's advised

Rule 3: Don't be TOO iterative

Another problem I've experienced is that a review can run for a very long time.  This is linked to Rule 2 in that, based on my review comments, people will make the changes and bring that file in that review up to date.  This leads to further comments and the whole thing just consumes too much time.  Crucible gives you the power to do this but you need a bit of discipline to simply know when to stop and when to start a new review.

In recent projects I have been performing a review, making comments etc. and completing the review (unless there is anything serious I want to make sure gets addressed ASAP).  When changes are made they generally get picked up in the next set of reviews anyway and as I talk to my team all the time I can give a brief sanity review when the changes are made.  

Making your submitted reviews lightweight helps with this.

Rule 4: Keep reviews small

Again similar to Rule 1 and 2.  Don't be submitting hundreds of files for a review.  If you need to make a big review considering breaking it down into smaller feature sets and describing this in the review.  It makes it easier for reviewers to perform the review and for yourself to make the changes quickly (if any)

Rule 5: Let everyone get involved

Gone are the days when a single (supposedly) smart person performs all reviews.  Social code reviews are and should be the future.  Embrace this.  Every single dev on my projects are added to reviews by default, even people with little to no experience of the technology.  Not only does it help people spot things that others may miss but it also 

  • generates debate and discussion around coding style
  • helps people get an understanding of the coding practise in the project
  • acts as a learning tool for people
  • helps people feel more involved in the project and team

Conclusion (cause I can't think of a better word)

Hopefully these guidelines give you a better understanding of how I work and how I get maximum benefit from Crucible.  After all it is just a tool and therefore open to abuse.  Honestly it's better than our previous approach (literally using Word documents) and I can't imagine anyone would prefer our old approach to this.  Does anyone else have any recommendations around the use of Crucible?

Published in Craftsmanship on April 18, 2011