Conducting Productive Code Reviews - Roundtable Discussion
May 20, 2003 - Boston SPIN
I was all set. I had the topic ready, my notes in front of me, and my leading questions ready. I even had the planned topics listed on the flip chart:
- Problems with Code Reviews
- Objectives of a Code Review
- Who should be involved?
- What happens: before, during, after
- Alternatives to code reviews
The only problem was that no one else was at the table. After a short wait, a couple of people sat down, the rest followed quickly.
We began the discussion with the observation that code reviews are not successful because no one comes to the meeting prepared--developers do not look at the code before the actual review takes place. What generally happens is that in the first part of the meeting, people sit in silence looking at the printouts until someone spots the first "mistake" which is typically not a logic problem. It was mentioned that what happens quite a bit in code reviews is developers argue about whether the code follows the coding guidelines and then migrates into a debate over bracing style. Most people around the table smiled and nodded in agreement. A good way to avoid this problem is to have a good code formatting tool, coupled with the policy that code should not be submitted for review unless properly formatted. Another common problem identified was that most new developers don't know what code reviews are all about, possibly as a result of lack of formal training. If there are any educators out there, please incorporate a section on code reviews/auditing/inspections in your programs.
The conversation flowed into discussions about how they have seen code reviews conducted. "Pair Programming", where one person codes and their counterpart reviews the code, was also a common implementation. This avoided the logistical scheduling complications of gathering a larger group and seemed to save time. One area where we did not really reach consensus was on timing of code reviews. The group was evenly split between having code reviews on a regular basis (as code was being written) and conducting code reviews in batches at the end of a programming cycle. We also briefly covered design reviews that are also beneficial but rarely held.
In our own organization (at MKS), we conduct design reviews and code reviews on a regular basis and have an open-door policy: "anyone is invited, but don't waste time if you don't have much to contribute." This does encourage people to do their homework before the meetings. Since I have not seen this done at many other sites, I wanted to find out who else had this type of policy and asked the question in the form of "Is open source code a good way to conduct code reviews?" My thinking is that the more people looking at code the better. An excellent point was raised: this works well if there is one arbiter.
General conclusions were:
- Code reviews are a good idea if people come to the meetings prepared
- Code reviews can be a great learning/training tool for new developers
- Establish and publish coding guidelines (enforcing would also be good)
- Define a simple code review process and stick to it.
I'd also recommend that if you are interested in learning more or improving your code reviews, take time to investigate the source code analysis tools that are available.
We ended abruptly due to time constraints so we weren't able to exhaustively cover the topic. Perhaps people would like to continue the discussion or recommend similar topics. Whatever your preference is, do not hesitate to speak up.
I did enjoy the discussion and look forward to participating again.
Steven Church, Product Manager, MKS Inc.