Code Reviews - Systematic Collaboration January 30, 2009
Posted by Allen Manning in : Process , 1 comment so farThe important point here is that adopting a trivial process or system can produce dramatic results. Once your grasp this concept and learn to apply it to your software projects, you can truly claim that you are working smart, not hard, and you can come that much closer to hitting your deadlines without the long hours and daily stress that seem to afflict so many software projects today.
Steve Maguire, "Debugging the Development Process"
Systematic
Steve Maguire is right, one of the keys to becoming a more efficient development team is to work systematically- devise automatic systems that ensure quality, and increase velocity.
The key with systematic, is in being automatic.
A professional soccer player who’s ability to trap a ball automatically lets her think about where she is going to pass it while it is in the air, rather than concerning herself with how she is going to gain control.
In our sport, the automatic systems we have in place frees up our mental space to worry about the hard problems- improved performance comes from a development a form of ‘body memory’.
Collaboration
Collaboration is key. The software we write is a multi-dimensional collaboration between us, our colleagues, and our customers.
Supportability
With our colleagues, we need to communicate ideas and support each other in solving difficult problems. As much as we may like to think so, there are no super-developers who have no need to collaborate with other developers and are islands unto themselves. Even if there were, we would want these Übermensch to go on vacation to cool down their mega-brains wouldn’t we?
Who supports their code when they are on vacation?
The key here is that our code must be both correct and supportable. Our colleagues must understand more of the system than just those areas they developed.
By collaborating and reviewing each other’s code we can detect stylistic differences and areas that need refactoring. Ideally, all code in an organization should follow a style guide and look relatively the same. When we review each other’s code, it should look like something we could have written.
You know if you are doing this right if after opening a particular class, you can’t tell who on the development team is the author. Stylistically it looks just like something that you or one of your colleagues developed.
This goes back to being ‘systematic’, rather than be distracted by the stylistic differences of the coding style, we should be able to concentrate on the problem at hand.
Defects
The cost of fixing defects increases with time, and exposure. The longer a defect exists in the software the more expensive it is to fix, and the higher the potential cost; by collaborating with each other and looking at each other’s code we find defects earlier in the process.
Knowing when we are Done
As mentioned before, there is no objective criteria to tell us when we are Done. Rather, we need our community to help evaluate when we have achieved Doneness. Code Reviews are a very contextual way for us to see exactly what is or isn’t Done and have the necessary discussions and debates needed to get us all on the same page.
Code Reviews
We can see why we need to collaborate and how working more systematically makes us more effective. By putting this together we find ourselves incorporating Code Reviews as a regular part of our development process.
There are plenty of tools for doing code reviews, and also some strong industry arguments for the benefits. Let’s focus on some practical advice for adding this to our regular daily workflow:
Review every changelist
Rather than the author deciding if a changelist is worthy of review, let the reviewer decide. The reviewer, more likely than not, is going to be the one supporting it so let him make that call.
It also removes the decision making process from the whole equation, there will be no time spent deciding on a criterion of ‘important’. Rather, trivial reviews will just be conducted more quickly.
Let the same reviewer re-review
If a reviewer suggests that some code needs to be changed, and the author makes some changes, then the reviewer should review those changes. This should continue for all follow-on changes.
This is a clean and systematic way for the reviewer to know what work has actually been done: If they don’t get any follow-on reviews then it can be assumed that no further work has been done.
Rather than define a system for what suggestions must be implemented, and which ones are ‘nice-to-haves’, it is lighter-weight to just have an ongoing dialog between reviewer and author.
Reduce friction by using a good reviewing tool
Rather than emailing to reviewers a diff or a changelist ID, use a Code Review tool that helps automate this process. This reduces the ongoing burden of looking up a particular change or navigating various tools to gain more context.
Crucible is a fantastic tool for facilitating code reviews, but there are many others worth evaluating.