How code review works

Let’s start with some basics, shall we?

Code review is the process by which the author creates pull request/merge request and assigns reviewers to examine it. Reviewers overview the code and leave their comments.

The next step in the code review process is working with comments. If the author does not agree with some claim, the author can reject it describing arguments to defend the point. Otherwise, the author makes the necessary corrections.

When implemented correctly, code reviews help developers discover common bugs faster and reduce the amount of work required to optimize code in the latter stages.

Benefits of code review

The code review process conducted correctly provides several benefits that are important as a whole and separately.

1. Propagation of knowledge

   Whenever more than one person is reviewing the code, knowledge of the solution starts spreading among the team.

2. Team ownership

   Code reviews have a positive impact on mutual code ownership. When the process of code review is spread out to the whole team, it's hard to end up in a situation where one developer always deals with a certain part of the codebase because they're most familiar with it. Knowledge doesn’t belong to a single person anymore.

3. Unifying development practices

   Not all programmers share the same code style, learning to read somebody else's code also means getting used to a different way of writing code and thus potentially being able to maintain it.

4. Code quality assurance

   Code reviews are of paramount importance to ensuring good code and providing a way to locate problems so that they can be fixed as early as possible.

Common pitfalls of code review

Code review is not always a perfect process. There are a bunch of common problems that companies face when doing code reviews.

1. Long wait time for feedback

   Process of waiting for review starts when the author opens a pull request and marks it ready for review. The waiting time could be 1 hour after the pull request has been opened (if you are lucky), or it might take a few hours or even days (if you are not lucky). There is almost always a bit of wait time again before the author sees the review and adjusts the code based on the feedback.

   This process can go on for multiple cycles and every handoff adds another bit of wait time. This is how a simple feature that was ready a few days ago still hasn't shipped yet.

   The wait time is excruciating because, after each wait time, the context is lost and needs to be rebuilt again. The longer the wait time the more difficult it is to remember how all the code changes fit together into the shippable feature.

2. Large pull request

   Large pull requests are pretty challenging to review. Especially if the code reviewer is not that familiar with the part of the code, reviewing can quickly become a nightmare. Study shows that a good pull request should not have more than 250 lines of code changed

3. Superficial feedback

   "Looks good to me" or just "LGTM" is a simple phrase that signals a job well done. The problem is that LGTM isn't always true when it comes to code reviews. Reviewers can use LGTM as a feint that gets the work off their plates as quickly as possible.

   There are several reasons why reviewers do not or can't give superficial feedback:

   • lack of proper technical knowledge,
   • not enough time to look thoroughly through the change,
   • misunderstanding the code.

   Also, structural and syntax errors are often the first things (sometimes the only things) called out in a code review. But what about deep-seated issues, like bad assumptions and faulty architecture?

4. Lack of time for proper review

   Doing a proper code review takes time. Another closely related problem is when teams want developers to do code reviews, but do not value or count developers' time spent on performing it.

What to look for in a code review

It’s important to go into reviews knowing what to look for in a code review. Reviewing code with certain questions in mind can help you focus on the right things. For instance, you might evaluate code to answer:

1. Design
   • Do the interactions of various pieces of code make sense?
   • Does it integrate well with the rest of your system?
   • Does this change belong in your codebase or a library?
2. Complexity
   • Is the code more complex than it should be?
   • Are individual lines too complex?
   • Are functions or classes too complex?
3. Tests
   • Does the pull request have a unit or end-to-end tests? In general, the author should add tests unless the code is handling an emergency.
4. Naming
   • Did the developer pick good names for everything? A good name is long enough to fully communicate what the item is or does without being so long that it becomes hard to read.
5. Comments
   • Did the developer write clear comments in understandable English?
   • Are all the comments necessary?
6. Consistency
   • What if the existing code is inconsistent with the style guide? If something is required by the style guide, the code should follow the guidelines.
7. Documentation
   • Did the author also update relevant documentation?
8. Context
   • Try to look at the whole file to be sure that the change makes sense.
9. Good Things
   • If you see something nice in the code, tell the developer, mainly when they addressed one of your comments in a significant way.

Look at every line of code that you have been assigned to review. If it's too hard for you to read the code and this slows down the review, you should let the developer know that and wait for them to clarify it before you try to review it. If you understand the code but don't feel qualified to do some part of the review, make sure there is a qualified reviewer on the code, particularly for complex issues such as security, concurrency, accessibility, internationalization, etc.

I’m learning and sharing: How to navigate code review

As a junior software engineer, I always analyzed code review comments I received to learn how to become a better coder. But when it came time for me to attempt my first code review, I realized my experience hadn’t prepared me to be on the other side.

At Unsizify, our pull requests are regularly required to be reviewed by either a peer or senior developer. I’ve been reviewing and have been reviewed (with various outcomes 😊) – here’s what I’ve learnt and what I would like to recommend as some of the best code review practices.

Be aware of the metric

Every pull request needs to live as short as possible. The longer it lives, the more problems it causes in the workflow: delays delivery of the feature/bugfix, increases a chance of merging conflict, annoys authors and reviewers who all have other work to do and don't like to return to an old task.

Avoid that spiral and instead, try to understand how your engineering team actually works. Gather valuable real-time data and make your decisions accordingly. What is the average lifetime of pull requests, average time for the first feedback, number of comments in a certain time frame, are there any long-running pull requests, etc. All these metrics can be a valuable indicator of your team’s performance.

Make it transparent

Pull request can easily become a ping-pong ball in a game played between pull request author and reviewer during its lifetime. With every new comment and code commit, pull request switches assignee from author to reviewer and back.

In these circumstances it can get confusing when trying to detect whose responsibility pull request at a certain moment is. Therefore, we’ve developed our own assistant to help us solve that confusing part.

Unsizify automatically matches the responsibilities of the corresponding team members by analyzing authors' and reviewers' previous actions. In addition, it sends relevant reminders through Slack notifications, just to be sure you got all the necessary task updates on time.

Conduct real-time reviews

Information that one reviewer is doing the code review at a certain moment could be, and should be, valuable to other team members as well. If another reviewer performs the code review, this would be a great time to comment on the code. If the reviewer is the 2nd waiting reviewer and someone else is also reviewing the code, this will prevent other reviewers from doing double the same work.

The complete real-time visibility of your team will help you to collaborate better throughout the reviewing process.

Closing remarks and takeaways

Code reviews are powerful means to improve the code quality, establish best practices and to spread knowledge. It’s more than just a bug-check. The code review process is critical for the success of any coding project.

Additionally, code review also serves as a guide for younger developers like me to sharpen their skills to become more efficient and knowledgeable.

My experience with the code review process has enabled me to become a better developer, both as a code reviewer and as the employee receiving code review. But even more importantly, well-run code reviews enable our team to balance the freedom to innovate rapidly with code quality and security.