The 10 commandments of navigating code reviews

If you've been in software development for any amount of time, chances are you've dealt with code reviews where the comments are harsh, blunt, or sometimes downright rude.

Code reviews are a form of feedback, but unfortunately they often lack the thoughtfulness that would be exercised if you were being critiqued on anything else.

Trying to force every code reviewer to be polite and thoughtful is a feat I personally don't have the energy to take on. I realized long ago that I cannot change people's actions, but I do have control over my reactions.

In my years of developing software, I've come up with 10 commandments that I follow to navigate code reviews. They have worked like a charm on every team I've ever been a part of, so I hope they work for you.

Testing in the Agile Era: Top Tools and Processes

I. Thou shalt not take it personally

You may receive nasty code review comments for various reasons. Perhaps the reviewer lacks the social skills to be able to give feedback in a constructive way. Maybe the reviewer takes pride in being brutally honest and lacks the time and desire to soften the message. Maybe the reviewer is just having a bad day. Or perhaps he or she is just a jerk.

Dwelling on why you have gotten rude code review messages is a waste of time and energy. You'll only become further frustrated. If you're in a corporate setting, you might have to take this issue to your manager in hopes that he or she will be able to fix it.

All of those reactions can make for a work environment that is even more uncomfortable for you. And if you're working on an open-source project, you could end up leaving the project and finding a new one, where there may be the same issues.

I’ve been coding professionally for 15 years and decided long ago not to take code review comments personally. Trust me, I have certainly gotten my fair share of rude or patronizing comments when joining a new team. However, they never last long, because I have learned to navigate code reviews and change the tone of them, no matter what the underlying issue was. Wondering why I am getting rude comments only stresses me out, so I have made the conscious decision to give every reviewer the benefit of the doubt and assume the comment is directed at the code, not me.

II. Thou shalt not marry thy code

One way to avoid taking code reviews personally is not to become married to the code. I know how much of ourselves we invest in the code we write. That passion is great. But if you truly want to commit the best solution possible, you have to be willing to accept that it's possible that the best is not your original solution.

Consider the following code review exchange:

Reviewer: This is complex for no good reason. You don't have to show off. Less is more.

Submitter: This is not me showing off. It's a complex problem and thus warrants a complex solution.

Reviewer: You can accomplish this same thing with a single while loop.

Submitter: My solution works just as well. You know, I really worked hard on this.

When someone looks at this exchange, the submitter is the one who looks emotional and defensive. The rudeness of the reviewer's comments is overshadowed by the submitter's attachment to the original solution.

When you detach from the code, it becomes much easier to have a conversation about the concerns addressed with the code.

III. Thou shalt consider all feedback

Once you've learned to detach yourself from your code, it becomes a lot easier to actually hear what reviewers are trying to say. You avoid playing the whole "Why are they being mean to me?" scenario in your head. I can't express enough how much stress this helps you avoid.

You can now read past the bluntness of the comment and see what point the reviewer is trying to make. Is the suggestion truly a better solution?

If it is, acknowledge it and make the change.

...

Reviewer: You can accomplish this same thing with a single while loop.

Submitter: Good point. That would be cleaner and remove some of the complexity. Let me rework this.

IV. Thou shalt articulate thy rationale

Many times, you probably do have the best solution to the problem you are solving. So, when someone suggests that you should change your code, don't automatically assume that person is right. You worked on this for hours, maybe days, and the reviewer is throwing ideas out there after seeing it for only a couple of minutes.

Consider the reviewer's suggestion, if you haven't already, but if you feel yours is better, articulate why. Don't go to the reviewer's desk and do it. Do it right there in the code review. You want other reviewers to know how thorough you are as well.

...

Reviewer: You can accomplish this same thing with a single while loop.

Submitter: Yeah, I initially thought the same thing, but I have to also iterate through the list. That led me to this solution, which handles all cases.

I've seen the rudest people change their tune after I was able to articulate why I solved a problem the way I did. I earned their respect and proved that I'm just as knowledgeable as they are in this particular matter. And I went even further by considering something they hadn't. If you're right, this is a sure way to get people to ease off in future reviews. They now trust that you know what you're doing.

V. Thou shalt be willing to compromise

Let's say you’ve considered the reviewer's recommendation but you know that it's not the best solution. But the reviewer doesn't think your solution is the best, either.

Don’t get into a heated back-and-forth exchange. After a few messages, if you cannot come to an agreement, it's time to step away from the code review. Offer to collaborate offline on the solution.

...

Reviewer: You can accomplish this same thing with a single while loop.

Submitter: Yeah, I initially thought the same thing, but I have to also iterate through the list. That led me to this solution, which handles all cases.

Reviewer: Well, there has to be a simpler approach than this!

Submitter: I'm all for the simplest approach possible. Let me find some time on your calendar so that we can whiteboard it out.

In this exchange, the submitter didn't crumble and blindly accept the reviewer's suggestion. And the submitter, not being married to this code, also didn't get emotional. Reviewers will respect both.

VI. Thou shalt contribute to others' code reviews

If you're always the submitter and never the reviewer, you're going to be perceived as more junior than you probably are.

Think about it: The most respected people on software projects are also the same people who frequently review the check-ins.

It doesn't matter if the submitter is more senior than you are. Even better! Think how powerful you become when you are able to help senior developers see something they didn't before, or introduce them to a new technique. This warrants instant dev cred!

It also allows you to see other coding styles and techniques, which, in turn, can make you a stronger programmer and more knowledgeable about the codebase that you are working on.

VII. Thou shalt treat submitters how thou would like to be treated

When you’re reviewing code, remember not to take on the same attitudes that you dislike. Just because someone is brutal when making comments doesn't mean you have to stoop to that level. This sounds obvious, but I've witnessed levels of pettiness that adults simply should not possess.

If someone just ripped you apart in your code review, it might be tempting to return the favor when you see a patch come through from that same person. Don't do it. Be the bigger person and stay true to who you are. Comment the way you would like others to. Who knows? Maybe your professionalism will rub off on them.

VIII. Thou shalt not be intimidated by the number of comments

It's easy to feel like a total amateur when you have a review with tons of comments. Things get even worse when there are multiple comments from multiple people.

I'll be perfectly honest with you: This doesn't look great, but don't count yourself out. Remember, it's code. Don't take it personally.

If you're new to the project or it's a new language, simply chalk it up as a learning opportunity. Go through all of the comments, review them, defend the solutions that you believe are right, and accept the ones that are better.

The key here is to communicate. I had to create a brand-new project in the Swift programming language, which I'd never used before. My first few submissions were magnets for code review comments, even from people who were junior to me. I was new to the company as well, so I hadn't yet established a reputation. It was crucial to me that I didn't become labeled as the emotional woman who doesn't know what she's doing. So I'd explain my rationale but in a nondefensive way.

Reviewer: Why didn't you just make the function's parameter optional and set it to a default value within the signature?

Submitter: Oh wow, that's pretty cool! I'm used to Java and we don't have that option. I'll make this change and read a bit more about it as well.

This shows that you’re grateful for the help and that you're not stupid—you're just used to another language. More importantly, you are willing to learn more.

IX. Thou shalt not repeat the same mistakes

Nothing annoys reviewers more than having to tell a person the same thing over and over again. I've seen code reviews become nastier and nastier when this happens.

When you accept someone's recommendation and make the change, take a moment and internalize why you accepted the recommendation. There's a lesson in there.

Again, make sure you’re not blindly accepting recommendations just to get people off of your back and get the review accepted. If you’re doing this, you may not be processing the "why." The "why" is the lesson and, once you understand why, you are not likely to make the same mistake in the future. So if reviewers are not explaining why they are making the recommendation, be sure to ask. Likewise, when you’re reviewing other people's code, always provide the rationale with your recommendations.

When a reviewer tells you something once, and you immediately start incorporating it in future submissions, people will also realize that you're sharp. They only have to tell you something once, and you get it. They'll respect that.

X. Thou shalt embrace the nits

There are people who seem like major nitpickers. Their only comments to you are about misspellings and extra white space.

If you’ve gotten to this point, congratulations! You’ve actually managed to write code with which your team has no fundamental problems.

Remember, don't take it personally. Don't roll your eyes in annoyance because it seems this reviewer just has to say something. Trust me: The reviewer more than likely respects your work and probably just has an eagle eye. Delete the stupid white space and keep it moving.

Obey these commandments

These 10 commandments are the gospel according to Angie. I keep them with me on every team that I work with, and they have not failed me yet. They have helped me go from newbie contributor to well-respected contributor in a matter of weeks.

What are some of your personal commandments for successful code reviews? Let us know in the comments.

Topics: Dev & Test

Popular Posts