What is code review and how to do it

If you have ever worked in a team as a developer, code review should be familiar to you. However, if you work alone or need a refresher, let’s start from the beginning to understand what code review entails.

Code review is a process where peers review and provide comments on the code written by a developer. It serves to improve code quality, readability, and also helps the reviewer gain a better understanding of the code and the associated feature or product. Additionally, code review acts as the first line of defense for quality assurance, as the reviewer can identify inconsistencies or bugs in the code.

Before examining the code, it is a good practice to test the modified code. This can involve clicking around on the frontend or making a simple request on the backend. These initial tests can help guide the reviewer in identifying potential code issues and what to be cautious of.

The next step is to examine the code thoroughly. During the code review, nothing should be overlooked. Sometimes, the reviewer’s suggestion may not be the better solution compared to another option, but any discussion is welcome. This helps both the reviewer and the author to think about the codebase and best practices.

When writing comments, it is important to be clear and concise. Comments that simply state “This is terrible” or “Please change” can confuse the author and lead to unnecessary additional changes. Instead, provide specific feedback or ask clear questions to ensure effective communication.

Additionally, it is important to consider the bigger picture. Keep in mind that you are not only reviewing a new slice of code, but also evaluating how that code fits into the entire project. This means that there may be instances where a line of code is not universally optimal, but it is the best choice for the project as a whole.

Task lifecycle and how we do it

If you have been working in an agile team, you are probably familiar with the task lifecycle. Once a task is created, it is added to the Backlog. When the team decides to work on it in a sprint, it is moved to the To Do column. Then, it progresses through the In Progress and Review&QA stages. Finally, when the task is completed, it is moved to the Merged and Deployed stages.

We can group these stages into 3 phases in our development process: pre-development, development, and post-development. Pre-development consists of two parts: Backlog and To Do. It is straightforward – when a task is selected for development, it is added to the To Do column.

The development process involves going back and forth between the “In Progress” and “Review&QA” stages. Once the assignee completes a task, it is sent for review and quality assurance. If any fixes are required, it goes back to the “In Progress” stage and the process continues until it is completed.

After the task has been thoroughly reviewed and confirmed to be free of any bugs, it is then moved to the “Merged” column. In this column, the task awaits its release and is prepared to be moved to the “Deployed” column. This transition marks the final stage of the task’s journey, as it is now ready to be deployed and made available to users.

As developers, we do not always have the choice to create tasks or put them in the “To Do” list. Additionally, even when someone starts working on a task, as a team, we may not have control over how quickly it will be completed. However, after that comes the Review stage, which can be seen as the Death of Productivity.

Sometimes, when you have a lot of work, you may forget to review something if it wasn’t a top priority task. The developer who was assigned to the task might remind you to review it after a few days. If this happens multiple times, the task can remain in the “Review” column for up to 10 workdays, even though it could be reviewed within 2 hours at most.

This leads to high-priority tasks being reviewed promptly, while other tasks are left in review limbo, waiting for someone to review them. This negatively impacts team productivity, as it appears that no releases have been made for a long time. Additionally, it is exhausting for the person assigned to the task, as they have to constantly think about it even after completing it and have to remind colleagues to review it.

Developer happiness

You have just completed a significant task and feel extremely satisfied with the work you have done. You eagerly send the merge request for review, expecting approval from your peers. However, as time passes – one hour, two hours, even a whole day – there is no feedback or approval. The initial excitement of completing the task begins to fade, and you move on to other pending tasks.

Just as you immerse yourself in a new task, comments on the merge request suddenly start flooding in. The previous sense of satisfaction now becomes a hindrance, impeding your progress and momentum.

As reviewers, we often overlook the fact that there is a person behind the code who invests time and effort into their work. It is not only important for them to receive timely code reviews, but also to receive constructive and concise feedback.

Here are a few guidelines to follow when adding comments:

How we boosted team performance

As mentioned previously, the most time-consuming part of the task’s lifecycle is the Review & QA stage. This is a process that developers can greatly influence. The key is to ensure timely reviews, so that tasks do not have to wait for several days before being reviewed and potentially requiring additional fixes.

We have decided to set up a daily Slack reminder that prompts us to “Review merge requests in the Review column.” While this reminder alone will not solve everything, as a team, we need to prioritize reviewing tasks right after our daily standup meeting. The purpose of this Slack reminder is to serve as a gentle reminder and encourage us to make reviewing a priority every day.

We examined GitLab metrics and found that, although they may not be entirely accurate (due to some tasks lingering in review for an extended period of time for reasons beyond our control), the average time from the creation of a merge request to its merging is 9 days.

It takes almost 2 weeks from the creation of a merge request until it is merged. We had hoped for a faster turnaround, especially for smaller merge requests, which ideally should take less than a day.

After one month of utilizing daily reminders on Slack and dedicating more time to reviewing merge requests, we have significantly increased our productivity. Now, almost all merge requests are merged within 3 days, ensuring that tasks do not collect dust in the Review column and enabling faster and more frequent releases.

The mean value for September and October was similar. However, the average value changed significantly, indicating a decrease in the number of extreme values (merge requests that linger in the Review Column).

In summary, after only one month we managed to boost our team’s happiness  and productivity. By giving it a thought and after discovering a flaw in our team where plenty of tasks were awaiting its review, our friendly reminders and daily dedicated time to task/code checkups, ensured that first, we got faster, but also better feedback; second, we dedicated time to be there for our colleagues and prompt team dynamics; and third, we eliminated the need of thinking about a simple task for weeks. Definitely, a thing to try if you recognised yourself or your team in here 😁

If you liked this topic, our docs-as-code blog might interest you!