DEV Community

Cover image for Better Code Review Practices
bob.ts for Leading EDJE

Posted on • Updated on

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


Smart EDJE Image

Top comments (6)

Collapse
 
andreasneuman profile image
andreasneuman

Absolutely agree!
If to talk about my experience with code review, recently I’ve found one nice tool - review assistant. It helps me to perform review better.
Do you use any additional code review tools?

Collapse
 
rfornal profile image
bob.ts

I don't actually use tooling beyond what GitLab or Gerrit bring to the table. I've actually heard of a few tools, including Review Assistant, but haven't used them.

Collapse
 
gsandec profile image
Gustavo Silva

This is a great piece of content. Thank you for your work here! I like the way you talked about the developer and the reviewer's perspective.

I decided to share a popular article on behalf of Codacy that is related to Pull Requests reviews. Feel free to read it here. Hope it adds value to this discussion.

Some tips from the content:

  • When you don’t understand what a part of the new code does, what do you do? You ask the author. And then he or she explains this part and then the rest. Now you’re reviewing the code with the author. You will inherit the author’s biases.
  • Your comments should be directed at the code, never the author.
  • Do the changes fit the application flow? Do they change the way something interacts with something else? Can something break? When you were learning to program, you created mental models matching what code does. So, as a developer, you kind of can compile and run code in your head. You are able to reason about a flow. You can apply this mighty skill to code reviews. Run the code in your head and wonder if it goes wrong. It could be the changed code or other parts of the application.
Collapse
 
brandon_hopen profile image
Brandon Hopen

Awesome article - its great to read your thoughts on the behavioral tendencies that are part of the feedback process.

On the knowledge retention side, have you looked at CodeStream? Would be interesting to heart your thoughts

dev.to/codestream/codestream-7-in-...

You can look at the full source tree when reviewing code, which should provide more context and inform how changes can impact other areas of development.

Collapse
 
cubiclebuddha profile image
Cubicle Buddha

Yes! This advice is so important. You have to balance the constructive feedback with positive findings. I treat code reviews like the mindfulness practice of gratitude. I try to think of one thing I like for everything I don’t like.

I really enjoyed your wonderful article.

Collapse
 
ppezaris profile image
ppezaris

Really appreciate your thoughts on the personal side of the code review -- IMO it's one of the toughest aspects to get right, and can often lead to some not-so-great interactions among teammates.