Please nitpick

30 Oct 2011 - ubuntu mozilla

When working with Open Source, some of the best learnings come from code reviews. When I first started coding on projects with strict code reviews Launchpad), I was uncomfortable getting nitpicked. I (wrongly) thought the goal was to have a code review that has absolutely no nitpicks (crazy right?). Whenever someone had a nitpick with my code, I felt like I couldn’t code properly. That’s when I read this excellent mail from Jeroen Vermeulen to the launchpad-dev team:

A rubber-stamp approval can save you minutes or more in the short term, but it does nothing for your longer-term development. A rubber-stamp approval leaves you without proof that the reviewer has read and understood the branch. A rubber-stamp approval sets no performance bar, no communicable standard for reviewers to live up to. A rubber-stamp approval does not tell you whether your branch was excellent, so-so, or too hard to read. A rubber-stamp approval buys you time that you could use towards the self-improvement you’re missing out on, but fails to tell you where you need it most. A rubber-stamp approval deprives you of a chance to harmonize your part of the codebase with our best practices. So I don’t like getting rubber-stamped any more than I like to rubber-stamp others. Good reviews take time, and that’s not time I put in for the sheer fun of it.

It changed my world. I’ve had epic nitpicky code reviews after that. Its even changing how I do my code reviews; I’m warming up to be more nitpicky. Lesson learnt – code reviews are one of the strongest takeaways from open source. So, if you’re reviewing code, please nitpick. Don’t rubber stamp. The professional development of the person whose code you’re reviewing depends on it!

PS: For acceptable levels of nitpick ;-)