I guess most teams do code review as one of the development cycles to make it better. However, have you experienced that your PR is not reviewed for a while? I have seen such a case. A PR that was open in December floated for 4 weeks and the PR is automatically declined by the system. This example is extreme but my PR is often floated for days.
Why does this happen?
Why should we start review within 24 hours?
Key of the code review
I think code review has 3 major roles.
- Improve the code for maintainability
- Share our knowledge to improve team members skills
- Specification check
What we implement is not always readable for other developers for various reasons. For example,
- The naming doesn’t fit the context very much
- The class cohesion is strong
- There are duplicated codes
- etc…
Reviewers might have a better idea to solve these issues. The code will be improved by discussing a better way.
During the discussion, their knowledge and thoughts are shared with each other. It is a powerful way to learn new techniques through code review because it is a good opportunity to apply them in production code rather than a practice.
The code author might misunderstand the specification. The specification itself might not be so good. It can be interpreted differently for a different person. In this case, the specification has to be discussed within the team. Therefore, the code needs to be double-checked to have the same picture and fewer bugs.
Why the PR is not reviewed
I start reviewing soon when I find it open and no reviewer is assigned. Thus, the list is just my guess.
Current task is not completed
All of us have a task. Some developers might want to start the review after the task is done. Is it clear when the task is completed? In an hour? By the end of today? In a couple of days?
We often face coding issues during our development process. Some of them are easy to solve but others not so much. If the root cause lies under one of the external packages, it takes a long time to find and solve it. If the reviewer is stuck in this hole, the review isn’t started.
A reviewer is not assigned
There are some cases that a reviewee opens a PR without assigning a reviewer. If the code knowledge is shared well, it might not be necessary to select one of the team members. This case happens if we think “I want to get the PR reviewed by who has time to do it.”.
In this case, the PR is not reviewed for a while until we directly poke one of them. Not many people want to increase their workload.
The PR is too big
If the task we are working on is not split into small tasks, the PR can be big. Not all tasks can be split well in advance because some tasks have some unclear things. We don’t know what we need until we do trial and error. While doing the trial and error, a solution is found with big code. In this case, the PR can be big.
If there are unclear things, some teams create a spike task for the investigation. But the code written in the spike task might directly be used for the production code and the PR becomes big.
If the PR is too big, a reviewer is probably overwhelmed. Where should I start reviewing? What do the class relationships look like? A bunch of issues arises if the PR is big. It means that it takes a while to review the code. After the feedback, a reviewee needs to fix those issues. However, the feedback loop might not be only once if it’s big.
This might be one of the reasons that a reviewer doesn’t start the review.
Working on higher priority task
The task might be a higher priority than the one for the PR. The developer working on the higher priority task can’t start the review in this case. If another developer is absent, the review isn’t started. If the team is small, we can’t make any progress except for the high-priority task.
By the way, there are only 3 developers in my team. Therefore, my PRs are often floating for a while if one of them is working on high priority task. This topic should be discussed within the team to determine how to proceed in this case.
Why within 24 hours
We should start code review within 24 hours after the PR is created. We all want to receive the feedback as soon as possible, right? I know that we all have a task on which we are working. When should we start the review then?
I propose that we start the review when it’s convenient. After implementing a function and the unit tests for example. If it’s on the way to implementing a function, keep doing it until it’s finished. But if it’s not done by the end of the day, start the review when we start working the next day.
That’s “within 24 hours”. If we finish a small task like implementing a function, it’s a good time to go to another task because we need to consider it differently when we start doing a different thing. After we fall asleep, it takes a while to remember what we did the day before. Then, start the review in order not to let the reviewee wait for a long time.
The reviewee might want to make the progress based on the PR. If the base code is not so good, the work has to be done again. The more work he/she does, the more wasteful it becomes. Even if he/she has a different task, the memory for the code will be old and it takes a longer time if a discussion is needed for the code. Discussion should be done while the memory is fresh because he/she can easily remember the code in detail. We (at least I) sometimes can’t remember why we implement it in that way. I personally need to read the code again if the review is done 5 days later after I open it because I work on another task as well.
If the code written in a PR is needed for another task but the PR has not been reviewed yet, we have to consider how to proceed with it. Don’t let the reviewee wait for your feedback. Let’s start reviewing within 24 hours.
Assign yourself if possible
Some don’t want to review the code that others implemented because it might take a while to complete it. But consider it again. Code review is a good chance to improve our skills.
We can know how other developers tackle a problem. There are probably new things for us. Naming, class structure, design pattern, some tools/packages that we don’t know. If we don’t know something in the code, we can simply ask why the code author applies the techniques/packages here. If we know the code well, we are able to write better code in the future because we can take those codes into account.
I review and open PRs more than half of total PRs. I think I know more than 80% of the code of my project. The percentage was not different between when there are 3 members and 6 members. I think I could break a task into small pieces better than other members because I’m familiar with the codebase.
Let’s be an active code reviewer.
Comments