Sunday, August 11, 2013

When code reviews become painful

Code reviews are good, see for example http://www.codinghorror.com/blog/2006/01/code-reviews-just-do-it.html.  This is not new.  In fact, I have experienced all of the benefits that come with code reviews.  They've helped me identify bugs, learn new ways to do things, learn what my coworkers are working on and all ... Overall, code reviews make the world a better place.

This past week, I found myself saying, "I hate code reviews".  I said it under by breath to myself so no one could hear it, but I said it nonetheless.  Here are two gripes I had with code reviews and some thoughts on improving the process.

Code reviews can lead to flame wars.  
My co-workers are great, and I have learnt a lot from them - yet, we all differ in the way we code.  The most frustrating code reviews are the ones where everyone agrees that the code works, but changes are needed for other reasons.  Maybe the reviewer thinks that the code goes against some best practices guidelines, the code will have some future maintenance issue or that the code is just unclear at first glance.   What do you do if the reviewer says, "Yes, your code works, but I would have done it this way."  Here are some things I thought of:

  1. Make your reviewer do some work.  If your reviewer critiques a piece of code, I think it is fair to ask for a suggestion on what they think should be done.  After a concrete suggestion is made, you can either implement the change or say why you don't think the suggested method is better.  If it isn't too clunky, a comment can be added with the alternative coding route.  
  2. Limit the number of time a change of code is discussed.  Unless there is a miscommunication, it should be possible to present all arguments for and against a change in two rounds of reviews.  After that point, I think the reviewer should accept the change or pass the code to someone else for review.  
  3. Don't get code reviews from your direct manager.  They have too much authority.  Their suggestions aren't suggestions, they become orders.  

Code reviews break my workflow. 
Waiting for my code to be reviewed takes time and breaks my workflow.  This is in contrast to great modern tools like automatic/continuous building and testing.  Immediate feedback increases productivity and delayed feedback is frustrating.

  1. When giving estimates for delivering an update, set aside another 2-3 days (calendar time) for each code change.  This will give you buffer time and will reduce stress and frustration.
  2. Make sure that you have a very realistic development and or alpha environment for testing where you can release code to before a code review is complete.  If you have an alpha environment, you can continue with the deployment process and ensure that if your change is accepted, you won't discover bugs later on when you are rolling the code out.  If the code review has a delay, you might find and fix bugs before the code review has even occurred.

No comments:

Post a Comment