Code reviews in the 21st Century
There's an old adage that goes something like: 'Do not talk about
religion or politics'. Why? Because these subjects are full of strong
opinions but are thin on objective answers. One person's certainty is
another person's skepticism; someone else's common sense just appears as
an a prior
bias to those who see matters differently. Sadly, conversing these
controversial subjects can generate more heat than light. All too
often people can get so wound up that they forget that the outcome of
their "discussion" has no bearing on their life expectancy, their
salary, their chances to win x- factor, getting that dream date,
winning the lottery, finding a cure for climate change or whatever it is
they regard as important!
Similarly, in the world of software engineering code reviews can end up in pointless engagements of conflict. Developers can bicker over silly little things, offend each other and occasionally catch a bug that probably would have being caught in QA anyway - that conflict free zone around the corner!
Now don't get me wrong, there are perfectly valid reasons why you may think code reviews are a good idea for your project:
- Catching bugs sooner means less cost to your project. You don't have to release a fix patch because it's has been caught in development phase - yippee!
- Code becomes more maintable. That crazy 200 line method that Jonny was writing with a hangover has being caughted before it has the chance to make itself at home deep in your code base.
- Knowledge is spread across your team. There are no longer large blocks of code that only one person knows about. And we all know, when that one person talks about taking a two month holiday everyone panics!
- Developers make more of an effort. If a developer knows someone else is going to pass judgement on his work, he's more likely to put that line of javadoc to clarify when an exception will be thrown.
- 1. Never forget TDD
- 2. Automate as much you can
When using these tools, it's important to use a common set of rules for them. This ensures your code is at some sort of agreed standard and much of what used to happend in an old fashioned 20th century code review, won't need to happen. Ideally, these tools should be run on every check in of code by a hook from your version control system. If code is really bad - it will be prevented from being checked in. Billy the developer is prevented from checking in the rubbish he wrote (when he had killer migraine) that he is too embarrassed to look at. You are actually doing him favours, not just your team.
- 3. Respect Design
- 4. Agree a style guide (and a dictionary)
- 5. Get the right tooling
- 6. Remember every Project is different.
- 7. Remember give and take
- 8. Be buddies
- Problems are caught very early
- You are always up to speed as to what is going on
- Reviews are always very short because you are only looking at new code since the last catch up
- Because the setting is informal - there is no nervous tension. They're fun!
- You can exchange ideas - regularly.
In summary, if your project is going to engage in code reviews, they should be fast, effective and not waste people's time. As argued in this post, it is really important to think about how they are organised to ensure that does not happen.
'Til the next time - take care of yourselves.
This scene happens at the Roxbury theme one night back in 90's. The original video can be found at YouTube.