Weekly Dev Tips
Weekly Dev Tips
Better Code Reviews
Hi and welcome back to Weekly Dev Tips. I’m your host Steve Smith, aka Ardalis.
This is episode 39, in which I'll talk a bit about how to make code reviews a little less painful of an experience.
If you’re enjoying these tips, please leave a comment or rating in your podcast app, tell a friend about the podcast, or follow us on twitter and retweet our episode announcements. There are millions of software developers in the world; help me try to reach a few more of them with these tips.
Better Code Reviews
I wrote an article about a year ago about Positive Reinforcement in Code Reviews. It generated a lot of feedback (on twitter if not in the article itself), so I thought I'd dedicate a Weekly Dev Tips episode to the topic.
Sponsor - devBetter Group Career Coaching for Developers
If you're not advancing as quickly in your career as you'd like, you may find value in joining a semi-formal career and technical coaching program like devBetter.com. I launched devBetter a few months ago and so far we have a small group of motivated developers meeting every week or two. I answer questions, review code, suggest areas in which to improve, and occasionally assign homework. Interested? Learn more at devBetter.com.
Show Notes / Transcript
I remember my first internship in college working in a professional software development team. It was at a big company not too far from campus. The building was a three story rectangle. The second floor was basically a giant single room - you could see out the windows on all sides. In the middle were cubicles. Hundreds of cubicles. One of these became mine while I worked there. I was given a work computer, a bunch of 3-ring binders full of documentation, and once a week there was a code review that I participated in.
I say I participated, but the internship didn't last very long and I spent most of my time fixing relatively simple bugs in C code, along with trying not to fall asleep while working through documentation binders. Thus, during the weekly code reviews, I mostly listened. These code reviews were conducted by the development manager. The week's updates were printed out and marked up by hand with questions and suggestions. From my perspective it was mostly a one-way conversation, though occasionally the more senior developers on the team would have a discussion about the code in question. The reviews weren't particularly insightful to me at the time, but the process itself stuck with me.
I understood the intent of the reviews - to up the quality of the code - but the process reminded me more of getting an essay back from a professor marked up in red than of teamwork and collaboration. And the relative infrequency of the reviews meant that, more often than not, the printed updates being discussed were no longer current anyway. A fact that often resulted in discussions about whether it was worth making recommended changes at this point, due to the rework it would require.
Over a decade later I found myself working at another company whose developers were required to conduct code reviews. This team's process was slightly different, in that each week a different senior developer took on the reviewer responsibility, and the reviews were done without any discussion or meeting. Once a week, whoever was in the reviewer role would go through the version control history and review all updates made in the last week (since the previous reviewer had done so). Any questions, suggestions, or changes were done in the form of TODO comments and emails, with management mandates that such requests be dealt with in a timely manner. It wasn't unusual, however, for deadline pressure to cause the review queue to build up, resulting in much more work to review or possibly in abandoning reviews for some period in order to get caught up.
In both of these cases, there were two major problems with the code review process. First, it didn't happen fast enough. Frequently reviews were looking at code that was days or often more than a week old, which on projects under active development meant that the team had long since moved on by the time the review was taking place. Second, the reviews felt more like the code author was being graded or evaluated than like the team was working together.
Fix this.
This is a bad name. Come up with something better.
You didn't follow the coding standard here.
So, what can we do differently today? Here are some quick tips - apply the ones that you think will work best for you and your team. First, do code reviews as early as possible. The earliest way is by collaborating while you're writing the code. I'm a fan of using pair programming especially on mission-critical code, and if you've ever asked for a coworker to take a look at what you're doing to help you out, you understand the value having another team member participating can bring. Code reviews can certainly still be helpful even for code written by a pair, but the pair should catch a lot of problems so early that you may not even realize it.
If collaborating while you code is too extreme for you or your organization, the next best thing is gated checkins. Many source control systems support this approach. My favorite at the moment is GitHub, who basically invented the idea and term pull request. A pull request is a conversation that happens about a change before it is made to the main source code branch. Teams I work with today use pull requests and reviews to ensure another team member looks over every PR before it's merged into the project's codebase. Usually the time from "hey can someone look at this PR for me" to someone actually reviewing it is under 10 minutes, because of notifications, Slack channels, and tight feedback loops.
Of course, maybe you don't want code reviews to happen more often. Maybe you don't want someone to look at your beautiful code - and maybe call it ugly - right after you've declared it's perfect. Part of that might have to do with how your team reviews code. Code reviews are an opportunity not only to catch and fix problems but also to encourage and recognize the good stuff. Positive reinforcement works, and can help make code reviews less painful and thus more useful, as well as providing another way to get your coworkers to write what you think is better code.
If a sincere accolade or positive message seems too cheesy for your tastes, consider easing up on the negativity by asking more questions and helping the author of the code arrive at a better solution themselves. Instead of saying "This code is a mess!" you might say "I'm having trouble understanding this code - could we work to make its intent more clear?" A related approach is to have a face to face or separate IM conversation with the author so you can have a more candid conversation that's not in front of the whole team (or company or world). As a rule it's better to offer praise in public but harsh criticism privately, and in any case it may be that you simply don't have all the information and a quick conversation will save you and the author a lot of trouble.
Show Resources and Links
That’s it for this week. If you want to hear more from me, go to ardalis.com/tips to sign up for a free tip in your inbox every Wednesday. I'm also streaming programming topics on twitch.tv/ardalis most Fridays at noon Eastern Time. Thank you for subscribing to Weekly Dev Tips, and we’ll see you next week with another great developer tip.
