Uncategorized

How I Learned to Stop Worrying and Love Code Reviews

 

Early on in my programming life that comic surfaced on the internet. It was actually right about the time I was working at my first internship writing production-ready code. It set the stage for code reviews for me, and honestly, it made me a little afraid of them.

For most of my career I worked on small development teams where I held a senior or lead roll. In terms of code reviews, this meant that I was often the one measuring the “wtf’s per minute” — I was reviewing other people’s work. Rarely was my code reviewed, and when it was, it was more of a knowledge sharing tool for large changes. Actually, when I look back at my past teams, we almost exclusively reviewed large changes to the code base. These reviews often took a long time and resulted in dozens of comments, which only extended the review process. So when I heard that we don’t push any code at Jana without a code review, I became worried.

First off, I was worried about how many “wtf’s per minute” I was going to produce. Would people be judging me based on code review comments? Would I have to defend every decision I made? How much of people’s time will I be wasting?

Then I worried about my own time. All these code reviews must take forever, right? When will I have time to write my own code?

As it turns out, there’s more than one way to review code; the way I was used to wasn’t always the best way. Here’s a few of the ways Jana makes code reviews constructive and less time intensive.

 

Size matters

Our pull requests are often small and contain work for just one feature. I would say most pull requests change fewer than 10 files. Small pull requests have a few benefits; it’s easy to spot changes that are unintentional, reviews are quick, and revisions get done quickly. Keeping pull requests small keeps the team productive and keeps code shipping.

 

Knowledge is power

When people hear that every change is reviewed, the first thing they usually say is “that’s overkill” — I thought the same thing. In reality, it’s a great way to share knowledge and keep a record of the work. When we close user stories or bugs, they always have a link to the pull request, which is useful when someone needs to revisit that story or bug. It also means that at least two people have seen the code and know what it does, which is very helpful when things go wrong.

 

Quality

At Jana, we don’t have a QA team. We not only rely on unit tests and integration tests to keep our app and services running smooth, but also our code reviews. Code reviews provide a great way to spot potential errors and dangerous code before it’s released. It’s also a chance to make sure the tests cover all cases.

 

It’s a discussion

Code reviews shouldn’t be frightening or embarrassing; they’re a chance to discuss and learn.  A comment on a pull request shouldn’t just be about the “correct” way, it should be a suggestion or a question as to why it’s being done a certain way. This may be a culture shift for some people, but it’s ingrained in the engineering culture at Jana. By not just telling someone to make a change and implementing it blindly, you learn more about the entire system and why the code was written that way.

If you’re also afraid of code reviews, I suggest you take a look at what your team puts into  pull requests. Try making them smaller and more constrained. Instead of telling people different ways to write their code, ask questions about how they arrived at the solution they used.  A few simple changes in your process can have a big impact on your engineering culture and your team’s perception of code reviews.

 

Discussion

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s