Better Code Review Practices

A CODE REVIEW is a part of the development process in which a developer and their colleagues work together and look for bugs within some code that may be ready for release. At such a moment, you can be either the code developer or one of the reviewers.

On one side of this process, you might not be sure of what you are looking for. On the other side, when submitting a code review, you might not know what to expect. This lack of empathy and wrong expectations between the two sides can trigger unfortunate situations and rush the process until it ends in an unpleasant experience for both sides.

Code reviews can be a powerful means to achieve higher quality code, establish best practices, and to spread experience and knowledge. It also lets developers learn from their peers, practice mentorship, and engage in open dialog and discussion about what they build.

However, when done wrong, code reviews can come to nothing or harm the interpersonal relationships of a team. Therefore, it is important to pay attention to the human aspects of code reviews. To be most successful, code reviews require a certain mindset and phrasing.

Here's how I have this organized:

1. My Code Review Experiences.
2. General Code Review Guidelines.
3. Code Reviews as a developer.
4. Code Reviews as a reviewer.
5. Who Should Review?
6. Positive Code Review Culture
7. The Subconscious Implications of a Code Review

My Experiences ...

End-User Code Review (no process)

When I started working as a developer, I remember having no code review process. In fact, the only review I got was a notification that some bit of code was not working correctly when a customer reported the issue. I learned very quickly that console.log did some nasty things with earlier versions of IE.

Everyone Stare At Me (weekly target practice)

From this, our team began an informal weekly process where we took some code and reviewed it while crammed into my boss's office. What a painful process ... having to explain design decisions on the spot in front of the entire development team; or even worse, trying to come up with intelligent questions about code in a language I had little familiarity with.

Everyone Reviews (learning to accept criticism)

The next team I worked with had a review process that was embedded into the commit and push toward the production process. Everyone on the team was included in the process, by default. The tooling in Gerrit actually made the process much simpler and quite intuitive; in fact, we were able to get a group put into Gerrit that allowed us to simplify the process of adding people as reviewers.

The first few times my code was pushed, the code reviews were painful (not having a clue what to expect) ... and I firmly believe I was learning almost as much about code reviews as I was generating code for the project itself.

How Is One Individual So Good At This?

We had an exceptional reviewer on this team that not only came up with exceptional insights into the code, he managed this in a way that made it feel painless. Because of this interaction, I have taken the time to investigate how I can duplicate this type of effort (the painless part ... the insights I know will come with time).

Code Review Guidelines

These guidelines stem from what a code review should accomplish. It is impossible to lay out guidelines that can be applied to every situation. Keeping these goals in mind will help achieve "the spirit" of code reviews even when a situation comes about that these guidelines don't cover.

Code reviews should:

1. Verify that the code is a correct and effective solution for the requirements at hand.
2. Ensure that the code is maintainable.
3. Increase shared knowledge of the codebase.
4. Sharpen the skill of the team through regular feedback.
5. Not be an onerous overhead on developer time.

As a Developer

For you, as the developer (or “author”, “submitter”), it is important to have an open and humble mindset about the feedback you will receive.

Anyone can do a code review, and everyone must receive a code review; no matter the seniority level. Receive any feedback gratefully as an opportunity to learn and to share knowledge. Look at any feedback as an open discussion rather than a defensive reaction.

We are human. And, humans make mistakes. This is completely normal. As long as the software is written by people, it will contain mistakes.

This does not mean that you should write code carelessly or stop writing tests. But this mindset will take away the fear of mistakes and create an atmosphere where making mistakes is accepted. This, in turn, is important for criticism during a code review to be accepted.

Exchange of Experience

During a code review, the developer and reviewer are exchanging best practices, experiences, tips, and tricks.

The developer and the reviewer are not only talking about the code ... they are exchanging best practices and experiences. Code reviews are a great means to establish and internalize good coding styles and best practices. And the exchange works in both directions. So consider code reviews as a valuable source of knowledge and an opportunity to learn.

As a Reviewer

Locating code to improve is a small part of a successful code review. Just as important is to communicate that feedback in a healthy way by showing empathy towards your colleagues.

As a reviewer, it is extremely important to pay attention to the specific language of the feedback. The phrasing is crucial for your feedback to be accepted.

Right: “It’s hard for me to see what this code is doing.”
Wrong: “You are writing cryptic code.”

Always formulate the feedback from a personal point of view by expressing personal thoughts, feelings, and impressions. It is difficult for the code developer to argue against personal feelings since they are subjective.

Before submitting a comment, remember to put yourself in the other person’s shoes. It is too easy to be misunderstood, so review the comment, always staying respectful ... speaking well to others is never a bad decision.

Take the Developer Out of the Feedback

Take the developer out of the feedback; only talk about the code that the developer is submitting for review. Criticism of the code is much harder to take personally because you are simply talking about the code, an objective thing, and not the developer. Again, this will improve the acceptance (as long as the developer understands that they are not their code).

Non-Judgmental Collaboration

The whole team's attitude and behavior should embrace non-judgmental collaboration, with the common goal of learning and sharing; regardless of experience level.

Accept There Are Different Solutions

Keep in mind that there are always different solutions to a problem. Most likely you have a favorite solution, but the developer's solution may also be valid. Distinguish between common best practices and personal taste. Remember that skepticism may just reflect personal taste and not incorrect code.

Remember Praise

It is totally acceptable to say: “Everything is good!”. No code changes is a valid outcome for a code review. Do not feel forced to find something wrong with the code.

And last, but not least: do not forget to express appreciation if the code is good. Praising never hurts and may motivate the developer.

Who Should Review

It is my humble opinion that everyone on the team should be included in the code reviews. Simply from a learning perspective, every team member gets to see the release descriptions at a minimum. I got to the point when in a hurry where I would simply check the comments made across reviews throughout the day. By watching I was able to see what changes were occurring. When I had more time, I was an active reviewer trying to help make the codebase better, one comment at a time.

Foster a Positive Code Review Culture

Code reviews done by peers can put a strain on interpersonal team relationships. It is difficult to have work critiqued by peers. Therefore, in order for a peer code review to be successful, it's extremely important that managers create a culture of collaboration and learning in the code review process.

While it's easy to see an identified defect as purely negative, each bug found is actually an opportunity for the team to improve code quality. Code reviews also allow junior team members to learn from senior leaders and for even the most experienced programmers to break bad habits.

Defects found in a code review should not be used to evaluate team members. If review metrics become a basis for compensation or promotion, developers will become hostile toward the process and naturally focus on improving personal metrics rather than writing better overall code.

Embrace the Subconscious Implications of a Code Review

The knowledge that someone else will be examining their work naturally drives people to produce a better product. This "Ego Effect" naturally incentivizes developers to write cleaner code because their peers will certainly see it. If your code is going to be reviewed, that is quite simply an incentive to double-check your work.

Conclusions

Code reviews are a powerful means to achieve higher quality code, establish best practices, and spread experience and knowledge. It also lets developers learn from their peers, practice mentorship, and engage in open dialog and discussion about what they build.

To be most successful, code reviews require a certain mindset and phrasing. Thus, it is supremely important to pay attention to the human aspects of a code review.

Leading EDJE