This is the story of the growing Khan Academy team converting me into a passionate fan of requiring a code review for every single changeset.
Those who have worked with me know that it’s a surprising position for me to take. On the spectrum of “Follows good development practices even if it slows down the product” to “Just ship the thing, code doesn’t matter, only users matter,” I tend to fall…right about…[furiously scribbling]…here:
Even though I’ve long been a fan of code reviews at both Fog Creek and Khan, I never would’ve suggested requiring them for every single changeset, no matter how small. At first glance it appears all P̝̂R̫͙̼̽ͪ̽̋Ö̝̹̿ͬ́̐̆̈CÈ̝̱S̮̜͙̩̠S̹͍̳̖͍̆̐Y̩̟̥̟̘̺̠ͭͫ̔, this is web development, not rocket science, and Hey what if there’s an emergency and Wait are you serious, you want me to review my single-line change to a trivial
Luckily for everyone, we’ve been hiring smarter and smarter people at Khan who can save me from myself. (What’s that theory about smart people and some lake? Lake Okeechobee, I think. With the crocodiles.) We told our team that we’re requiring code reviews for all pushes a couple weeks ago. Spoiler alert: it’s not that processy, and even a Just Ship It clown like me is already seeing immense value from the experiment.
Croc! No, sorry, wait…log!…Or, no! Wait! Sorry…Croc!
All changesets reviewed.
It’s not for every group. I’m not convinced it would’ve been right when Jason and I were hacking together alone. But requiring code reviews has already made for a better product and a healthier team, two things that I personally care about far more than a healthy codebase. That’s just a nice side-effect.
P.S. When working on Kiln, I was adamantly opposed to building code review requirements directly into the source control product. I stand by that belief, and I’m almost certain the Kiln team still agrees. Reviews should be required by your team’s dynamics and strongly encouraged by your tooling, not the other way around.