In SML we do code reviews. We do them on daily basis. Actually the point that we are now is a result of long journey that we made. We try different strategies and tools until we went to the place that we are now (but it doesn't mean that we are going to end up here).
During this journey we found many risks and traps that are waiting for a newcomer. That's what this post is all about, traps & misconceptions on code review.
Code control: many organizations uses CR for controlling codebase. Most of them are using pre-commit strategy. In many cases its because those projects are open-source with hundred of commiters. In real life this is quite rare scenario, so if you hired someone it means that you trust him enough to let him commit code to repository. I know that in some organizations there will be temptation to make procedure that will force developers to "review" and "approve" every commit, but it will not guarantee the quality. Moreover people will soon treat code review as "stupid" corporate procedure and will try to hack-it (such changing password every month e.g. people are using passwords like: mypass1, mypass2 etc.).
Hall of blame: Don't user CR for finding scape goats or guilty ones. Let's assume that there was a failure and you found a person who "reviewed" the "bad code" and blame him for not pointing it out. I will cause that development in your company will drastically slow down. People will be pointing out every semicolon not at the right place, because they will be afraid of being scape goat. Your team members will start feeling unconfident and the lack of trust.
"Code" of duty: Don't push on your developers to much. If you force them to make review every day for an hour, soon they will hate it and treat as unfunny duty. Code review is about learning, praising others and giving feedback so it's very social activity. CR could be fun, don't spoil it.
I'm not my code: If your code was reviewed by someone and he left some comment (sometimes even not too nice), don't get angry. He isn't saying that you are a poor developer. That wasn't his intention nor code review is about that. All he did, was criticizing some piece of code (not the author). Code review is all about code, not about you. Don't treat code review tool as forum with "trolls" and people fighting each other. When you will be writing comments try not to be rude or too strict. Try to imagine that your are on the other side reading it.
To sum it up, there is plenty of ways to make code review wrong. Those 4 are the one that I experienced or anticipated, so beware. I promise to write post about "what code review is about".
If you wan't to try out code review tool (fruit of our experience & practices) visit codebrag.com
If you wan't to try out code review tool (fruit of our experience & practices) visit codebrag.com
Used photos:
1. "The Puppet Master" by Henk
2. "ready for duty" by Leonard John Matthews
3. "Polar wolf's argument" by Tambako the Jaguar
2 comments:
Hi, interesting post! Some time ago I also blogged about code reviews but from very different perspective. You might find it interesting: http://www.kaczanowscy.pl/tomek/2012-08/code-reviews-mindmap
Cheers!
Hi, I just read your post. I don't agree with everything but I've got similar experience.
Especially making review on public could be risky. Keeping distance to your code is difficult and even more difficult when it's publicly criticized. On some cultures even non acceptable.
The other problem is a lack of good tool. That's why SoftwareMill is making Codebrag.com
There is couple tools worth consideration but flow of non of these is simple or intuitive. So If you would like to try beta or became Early Adopter of Codebrag. Just let me know or write ask@codebrag.com.
Post a Comment