Online Code Reviews suck – even Guido van Rossum can’t fix that
Guido van Rossum has created a new tool project called Mondrian for online code reviews. But even Guido can’t solve the fundamental problems of an online code review. Nothing beats face to face communication. Code reviews are not just about finding bugs in code, they’re about conversations, mentoring and knowledge transfer (so everyone on the team understands the code).
Guido appears to have built a very slick system for getting other developers to read and comment on your code. It manages all aspects of the review process but doesn’t around the fact that face to face communications have much higher bandwidth than text.
In person we’re not limited to staring at the code (or diffs) we can
move away from the computer and us a white board. We can shift from low
level issues (like why is this member variable public instead of
private?) to higher level issues (like is it possible to decouple these
classes?).
Finally it almost begs why isn’t google doing more pair programming
combined with automated tests? If you’re coding with a pair partner why
do you need another review?
If you want something automatic try static analysis tools like PMD
(for Java) or FxCop (for .NET) that are able to spot a certain class of
coding mistakes. For the limited set of things they do they’re far
Thanks to Niall Kennedy for posting about this.
If you enjoyed this post, subscribe now to get free updates.
Update this post is featured in the Carnival of Software Development
Related posts:
Did you love this post?
Then please share it!
June 7, 2007 at 11:43 pm | Stacia Broderick
I’m not a developer, but the notion of a code review tool (Crucible) was brought up in my course today by a developer. Just the thought of it made me cringe… I couldn’t possibly see how a tool would be able to do a better job than TPAWB (Two People at a Whiteboard). I couldn’t convince the developer that two people talking about and reviewing code together, in person, in a dialogue is much more effective, and less overhead than having a tool do it for you. He said, ‘yes, and the tool helps us keep track of what we’ve reviewed’. So, a question for you: do you think that a tool would be useful when tracking code reviews for multiple teams on a project? I tend to go the trust route; if my teams have an acceptance criteria on a user story that says ‘code review’, I’m going to trust that they’re doing this. But, considering the case of tracking multiple teams’ code reviews, is a tool helpful? Or is it just wasteful overhead? Thanks for any help you can give!
June 8, 2007 at 9:37 am | Mark Levison
Stacia – Trust is a funny thing – why didn’t your student trust his team? What additional value does he think that he will gain by tracking the reviews? On my teams we have a simple rule – work gets reviewed. Either every checkin or when your ready to declare task done. Work isn’t done when its coded, it needs to be reviewed and tested by some other than the developer (preferably QA). Once you’ve established this rule you don’t need to track what’s been reviewed – since everything gets reviewed. If you have legacy code then your responsible for improving the area you’re working in every time you do some work.
But far more interesting why doesn’t this person trust their team?
July 4, 2007 at 2:40 am | craig
i hear what you are saying re-face to face, but the problem with this style of code review is also its strength. it is point-to-point by nature. If you are not in the review, you cant learn from it. Moreover, it assumes that developers are in the same desk, geography, etc. This is often not the case. I work in a lab on a product that is separated into labs in the US and in Sydney, Australia. Whilst there is strength in having face to face meetings and phone calls…I believe there is a place for an online, collaborative code review format. In our own lab, we have problems scheduling reviews that include the other lab because the timezones are never ‘ideal’ (too early in the morning or too late).
Many shops are implementing ‘follow the sun’ development. This format of review allows for folks to review and collaborate in their timezone, which I believe has advantages.
February 11, 2007 at 3:12 pm | Sharp Blue
The Carnival of Software Development, number 2
Welcome to the second edition of the Carnival of Software Development. This time around there were many more submissions than…