POSTD’s article [Translation] Practical Lessons in Peer Code Review contained many points that could be incorporated into actual work.

I’ll record quotes and comments here as a memo.

Why Do Code Reviews?

First, let’s remember why we do code reviews. One of the most important goals for professional software developers is to constantly improve software quality. Even if your team members are all talented programmers, as long as you work as a team, you need to think separately from being competent freelancers. Code review is one of the most important ways to improve software quality. It’s particularly important in the following aspects:

• Gain objective perspective to find bugs and better approaches.
• Ensure at least one other person besides the author understands the code.
• Viewing experienced developers' code helps train newcomers.
• Both reviewers and reviewees can share knowledge by exposure to other members' good ideas and practices.
• Knowing they'll receive colleague reviews makes developers strive for more complete work.

I agree with all the important points listed in these bullet points.

Code reviews have costs, but in projects involving nearly 10 engineers and designers, the benefits outweigh them. The product I’m currently involved with used to push ahead with product development without code reviews for non-critical features when there were only a few developers. However, now that we’ve reached a scale of 10 people, I feel that phase is over.

Reviews are a useful means to reduce the probability of critical problems.

Conducting Thorough Reviews

However, the above goals cannot be achieved without conducting reviews with appropriate time allocation and due attention. Simply glancing through a patch and checking if indentation is correct and all variables use lowerCamelCase is not thorough code review. It’s beneficial to consider the commonly used practice of pair programming. In this approach, code review takes basically the same time as development, making the total time 100% more than the overall development time. Compared to pair programming, no matter how much time one engineer spends on review, it’s much less, so you should be able to spend sufficient time on code reviews.

In my opinion, spending approximately 25% of the time spent on original development on code reviews is reasonable. For example, if you spend 2 days on development, you should spend about 4 hours on review.

When reviewing code, you should check not only for bugs and other problems in the code but also:

• All necessary test cases are complete.
• Appropriate design documents exist.

The idea that “spending approximately 25% of the time spent on original development on code reviews is reasonable” is something I use as a guideline when calculating development effort.

Preventing Code Review Overload

When teams mandate code reviews, there’s a risk that the code review backlog could grow to unmanageable levels. Even if you don’t conduct any reviews for 2 weeks, securing a few days to catch up on review delays isn’t difficult. However, in this state, when you actually conduct code reviews, development work can get caught up in major unexpected problems. Also, maintaining proper code reviews requires maintaining a mentally focused state, making it difficult to improve review quality. It would be hard to maintain this state for days on end.

Therefore, developers should strive to clear the review backlog daily. Conducting reviews first thing in the morning is one way to handle this. By doing pending reviews before starting the day’s development, you can escape the situation of developing while constantly thinking you need to review. Some people might want to do reviews before or after lunch break, or at the end of work. Whenever you do it, by considering reviews as part of your daily work rather than as an obstacle to development, you can avoid things like:

• No time to clear the review backlog.
• Releases delayed because reviews aren't finished.
• Commenting on old code when the code content has already changed.
• Insufficient reviews due to rushing through them.

Starting last week, I’ve adopted the habit of “reviewing first thing in the morning.”

With nearly 20 Pull Requests needing review, which is an unhealthy state, I think it’s necessary to review and merge consistently every day to keep the product healthy.

Needless to say, 20+ pull requests is far too many - I want to get it down to a number you can count on one hand.

Writing Reviewable Code

If there are complex aspects to the architecture, meeting with reviewers beforehand to discuss them makes sense. This allows reviewers to understand what the code is written for and how functionality will be implemented, making code easier to understand. For code writers, it also helps avoid situations where they have to rewrite large portions of code after reviewers suggest different, better approaches.

It’s important to discuss specifications with reviewers and team members beforehand to prevent major rework at the review stage.

Large-Scale Code Refactoring

The best solution is to gradually refactor code.

In addition to this, when doing large-scale refactoring, you should always share information with team members that you’re starting refactoring work.

Resolving Controversies

Your team is undoubtedly composed of intelligent professionals, and in most cases, you should be able to reach agreement even when opinions differ on specific coding issues. As a developer, be prepared to compromise with an open mind in case reviewers prefer different approaches. Don’t take a possessive attitude toward your code. Also, don’t take review comments as personal attacks. Just because someone feels you should refactor some duplicate code by reusing other functions doesn’t mean they’re denying that you’re an attractive or wonderful person.

Be considerate as a reviewer. Before suggesting changes, carefully consider whether it’s truly an excellent suggestion or just a matter of preference. You’ll do better by choosing which points to discuss and focusing arguments on parts of the original code that clearly need improvement. Instead of saying “My pet hamster could write a more efficient sorting algorithm than this,” say something like “It might be worth considering ~” or “Some people recommend ~.”

If you can’t find a compromise, it’s good to have another developer whom both parties respect take a look and give their opinion.

While taking pride in your work, you always want to maintain a humble attitude.

Personally, as a reviewer, what I need to be careful about is whether I’m pushing “mere matters of preference.”

Code review is deep, so I’d like to compile an article once I’ve accumulated knowledge through practical experience.

That’s all from the Gemba.