Not every developer approaches code review the same way. For instance, some people consider good naming essential while others argue that they don't have time to discuss naming. My take on that is clear: Naming is difficult but important, shapes thinking and helps understanding of code. It's okay to not get names right the first time but if you don't have time to fix bad naming during review, your team is probably in trouble.

I have documented a team's approach to code review more than once so maybe it's time to share my approach to code review as a blog post.

Why do code review?

Why is time spent on code review time spent well? Code review helps…

• to transfer knowledge (both ways),
• to keep code quality up and to prevent introduction of bugs,
• to reduce bias on where to take shortcuts and where to go extra miles, and
• to practice communication.

How to write code that is about to be reviewed?

My rules of thumb for making changes and commits are:

• Make things that explain themselves.
• Comment what cannot explain itself.
• Resist adding unused features.
• Add tests for every bug you fix.
• Add tests for every feature you add.
• Keep code coverage up.
• Keep technical debt down.
• Make refactorings, bugfixes, and whitespace changes go into separate commits.
• Clean-up while going, in healthy amounts.

How to do review?

During review I am looking for…

• introduction of new bugs,
• things that were forgotten but should not be missing,
• things that are misleading or easy to misunderstand,
• places that cause more cognitive load than necessary,
• things that do not explain themselves and are not commented; applies on all levels, e.g. variable names, class hierarchy, etc.,
• unclear and/or lacking commit messages,
• commits that do more than one thing or more than they should; good Git history helps understanding the past so it better be clean,
• inconsistency with the existing code base,
• common struggles, e.g. proper use of super() in overridden methods.

I am not, or at least a lot less, reviewing for and caring about:

• code formatting — we mostly have CI and pre-commit checks for that —, and
• enforcing the same style on everyone.

Challenges during code review

Things can go sideways when…

• code review starts to replace proper testing,
• authors expect waving through rather than proper review,
• review becomes personal,
• attitude "we do not have time to discuss this" meets someone who cares,
• code quality remains low across review iterations so that the reviewer could have done the whole thing him- or herself better in a fraction of the time and energy,
• pull requests are too big to begin with,
• unreasonable time constraints exist,
• there are larger gaps in knowledge on the side of the reviewer,
• non-obvious claims are made but are not backed up with sources,
• reviewers request clean-ups that exceed the scope of the change and the energy of the author, or when
• multiple reviewers disagree and insist on their point.

Do you want to discuss your code review situation with me? Please get in touch.

Best, Sebastian