Code review, an important process within a development team

Disclaimer: Whatever you read here is just based on my experience and is my personal opinion! You may have a different idea!

Why do we need code review

There are many reasons to review code, and I am sure you can do your own research to figure it out, but here are some reasons that came to my mind at the time of writing this article:

  • Share knowledge
  • Catch accidental errors
  • Check test coverage
  • Check consistency
  • Quality control

If you are a developer and finished your task, it is your responsibility to find someone to review your code. Your task doesn't finish after you create a pull request. Obviously, this depends on your team and you may have a different approach.

Two extreme scenarios

Team 1: Very strict
I have worked in teams that people were very strict in code reviews and spent a lot of time going through all changes and understand them. They were checking everything, like naming conversion, indentation, and even extra spaces.

Team 2: Very easygoing
I also worked in teams that people were very easygoing and used to approve most of the pull requests quickly.

As a developer, my preference is the first team (I had a different opinion a couple of years ago!). At the end of the day, the quality of the code is very important. Having said that I was curious to know why people in the second team did not take quality time for code reviews.

The people side of the things

Throughout my career as a software developer, I have seen different cultures in teams when it comes to code review. Firstly I think code review culture, is very much dependent on the company's culture. If there is not a good team binding, code review can become a place to argue and blame. People want to prove they are right not others. However, a good culture can lead to a more constructive review process and eventually a better quality of code.

You are a reviewer, how do you put comments on code?

First of all, as a reviewer, you should not sacrifice the quality of the code. Don't stop providing feedback to other developers. However, you should think about the comments that you put. Sometimes a pull request comment can go wrong and makes developers frustrated. Here are some tips:

  • If you are not sure about something, don't make the pull request author do the research for you! You better do your research first and then push your idea forward. Alternatively, you can just suggest an optional change, if the developer is keen to investigate it.
  • People can interpret a single comment differently. It is very much important to know the person who wrote the code. Some people are eager to accept feedback but some people may not be and can take it personally. I think if you are not sure how a developer might react to your comment, go and talk to him/her instead.
  • If you cannot reason about your proposed change, don't put a comment. People have different coding styles and unless there is a reason, like performance, code standard, clarity, etc. you better not ask people to change their code.
  • You should not only comment on things that you think are wrong. If you see a positive point, mention it, please. For example things like "I did not know you can use this function here" or "I think this class has a good test coverage, well done!".
  • Choose your words wisely. For example instead of saying "Why did you use this function? Didn't you know it is slow?", you can say "Last time I used this function it was slow, was there a performance improvement in it recently?"

What else can impact code reviews?

  • Sometimes external factors can impact code reviews, things like deadlines, or stakeholder pressure. A reviewer who is under delivery pressure may spend less time on code reviews. Also, he may skip asking for changes while reviewing others' code in order to not block the delivery.
  • Lack of visibility for the whole team. If there is a large pull request and some people need to spend a considerable amount of time to review it, you better have some visibility in the team for that. For example, you can add a task to the backlog item only for the code review. In this way, the reviewer can spend a good time reviewing and everyone else can see it.
  • I also noticed some people want to be nice to their colleagues, so they avoid putting comments on pull requests as much as they can.

How long should a code review take?

I think it is important to make it clear to your team that code review is part of the development and they should take whatever time they need to. But make it visible, create a task for large pull requests so that everyone in the team can see the effort.

19