Google Code Reviews
code reviews, software development
Carl Quinn, a development manager at Google and a member of the Java Posse, mentioned that he spent about 50% of his time on code reviews. Google even has a special code review tool, Mondrian, developed by Guido van Rossum. The features are similar to Crucible.
Google’s warm embrace of lightweight code reviews sets a nice bar. We’ve heard for decades that code reviews or inspections are great for removing defects, keeping the code base maintainable, and mentoring. But…
- There’s no time, we have code to write.
- I have a healthy ego, I’m not comfortable with my peers giving me feedback.
- My code already works, it even passes those unit tests I wrote at the end.
- Code reviews are just about individual style, not worth the time.
- We’ll pair program then we don’t have to do code reviews. Oh, you mean I have to sit with another developer and share a machine? Let me think about those code reviews again.
- Code reviews, those aren’t Agile.
Can Google’s example win over more developers?
Ed Gibbs @ July 6, 2007
Second try to add this comment. This time I will save a copy just in case the system rejects me again.
Ed - Any code reviews are good - but I think that online code reviews miss most of the value because there is no face to face conversation involved.
See: Online Code Reviews suck even Guido Van Rossum can’t fix that: http://www.notesfromatooluser.com/2006/12/online_code_rev.html
We do a bit of a combination with our Crucible based code reviews. People do the reviews online and if any significant design questions crop up we get together and have a meeting about it.
We’re still adjusting our process. We’ve tried printouts, or projector displays of the code and holding meetings. The problem was no one prepped or looked at the code until the meeting. We’ve also tried at times reviewing all the code, but generally it’s been too much. Effectively we’ve found a developer can review about 400-600 lines of code in about an hour. Beyond that people get a bit overwhelmed.
Things we haven’t tried yet are pair programming except in some very limited cases and doing buddy code-reviews where you call over another developer just before checking something in and go through the code with them making any changes needed right then. I like the idea of both of these approaches, but I’m still working on developer buy-in.
I do appreciate the perspective of in-person reviews.
We use exactly the later approach. No checkin happens until you’ve found a reviewer. The reviewers name gets etched in the checkin notes.
Developers are also expected to rotate reviewers so that you don’t default to the same set of eyes.
Cheers
Mark