Code Review - Check yourself

This section of code review questions are related to self-awareness and reflection. As long as programming remains part art, people will be attached to the code they write. This makes reviewing code difficult. Being too attached to code can make a person be defensive and unwilling to change. While it is the author’s responsibility to detach themselves from their code and not be defensive, it’s the reviewer’s responsibility to be empathetic, kind and respectful.

Should you have a talk with the developer before reviewing the code?

There could be a number of reasons to talk with the developer before reviewing their code. Perhaps the relationship is new or strained and you need to build/rebuild some trust. You may also need to talk with them if you don’t understand the change. Or maybe the change is massive and you could benefit from some insights on how to approach it.

In each of the scenarios above, the conversation is going to be different. If you find yourself struggling to have any of those conversations, then that’s an area you should look to improve in. Seek out resources within your organizations and outside of them. Read books, ask questions and practice.

Should you have a talk with the developer before submitting the review?

The most difficult thing in software development is communication. If you find that your review has a number of foundational questions about the change, you may not understand the change. Or maybe the developer misunderstood the requirements. In this case rather than submitting a large review, have a conversation to understand the purpose of the change and the reasoning that went into the developer’s choices.

This may feel like a waste of your time since that review may never get sent off, but the goal isn’t to have each step of the Software Development Lifecycle be productive. The goal is to produce quality, reliable software consistently and sustainably. A review that permanently displays a misunderstanding or miscommunication could undercut a developer’s self-confidence.

Keep in mind this is going to vary from person to person and organization to organization. I only ask that you do some self reflection and try to empathize with your colleagues.

Are you leaving too many nitpicky comments?

A nitpicky comment is one that’s regarding a style choice or trivial code change. These can be important in the grand scheme of things and they need to be dealt with, but you need to be aware of the tone and message a review sends.

If there’s a repeated problem in a change, write a single comment that indicates what the problem is and let them know there are other cases as well. Leave it to the developer to identify the remainder of the cases. After they resolve it, confirm that all the cases have been addressed.

If you prefer a different style, say '' strings rather than "" strings, utilize a linting tool that will automatically transform the code to conform to the style. I agree that consistent code is easier to read, but we shouldn’t try to force a developer to write code a specific way that only differs in style. Let a person do what’s comfortable for them and they’ll be more productive. If there’s a good reason to avoid something have that conversation. Explain it and be gracious.

Are you trying to make the other developer write your code?

If you’ve delegated some work to another person they are going to do it as they feel is best. They have their own experiences, beliefs, habits and preferences. This means they will likely not approach it the same way that you do. If they implement a solution that’s reasonable, but not perfect, is it good enough to be used?

If it’s a small thing and doesn’t have cascading effects, I like to acknowledge the solution and let them know that it’s good to go, then bring up the way I would have approached it. I let them know that they need to consider the project’s timeline and effort involved before changing anything further. I prefer to leave it to them to make the best decision possible.