Why We Code Review
Code review can be an important part of a team’s culture, so it is worth thinking about. If you asked me about code review two years ago, I would’ve said its key to maintaining code quality and mentoring less experienced programmers. Now I know better. Code review is much about making the code better as it is about making the team happier. How the team runs code reviews should be based on the team’s values and culture.
The obvious things
Let’s talk about the obvious things first. What are we looking for out of code reviews?
- Code quality
Because good code reviews allow a team to maintain high code quality.
- Consistency
Where you have a non-trivial group of engineers writing code together, it is important to develop consistency. This applies to both style (tabs, naming conventions, patterns & anti-patterns) and tooling (test frameworks, code generation, build tools). When there is consistency, everyone’s productivity improves.
- Best practices
The “best” in best practices is misleading. But “At Least It’s Not Bad” practice doesn’t roll off the tongue (try selling that to management). There are some good rules of thumb in programming and it’s important to get everyone to follow them. Code reviews help ensure everyone generally do the right thing.
I’d consider these the minimal goals when you are fostering a code review culture.
What are your values?
Show me your code review guidelines and I’ll know what your team is like.
If you think about it, code review is all about conversations. It starts with “I have good code and I want to share it with the team” and ends with the saying, either “No. This code is shit!” or “Hey, let’s talk about this.”
Whether you’re trying to fix a broken code review process or introduce it in your org, your team should have some ground rules around how to have these conversations. Of course, there no wrong way to have these conversations. They may be brutally honest (euphemism for “we don’t care about each other’s feelings”) or polite to a fault, but they are always a reflection of the team. If you’re in management, they are a reflection of how you’ve shaped your team.
Just like every family has its unique characteristics that make them special, every team conducts code reviews differently. There are some key areas with very different variations:
- How to take people’s feelings and assumptions into account?
- How to signal a strong objection vs a weak one?
- How should people resolve disagreements?
- Do they value individual code ownership or group ownership?
A clear set of principles help the process run smoothly and amplify your strengths. They will change as the team grows. So keep in mind to revisit these discussions as necessary.
Some of the important cultural elements of code reviews:
A good way to say ‘nitpick’
How does the team deal with nitpicks? From silence to ‘nit’, each team has its own unique ettiquette. One method to silent nitpicking is by having linters and static analyzers do the nitpicking. When’s the last time you got upset by your spellchecker pointing out your mistakes?
What is the acceptable level of code coverage?
Some teams value test coverage. Some teams don’t care a bit. Some teams care but don’t make them blockers. Even worse, this decision can vary based on the specific parts of the codebase. It might be that your team expects high coverage on code written for a new system but is ok with lower coverage on a legacy system. This is very subjective and situational, so you should take care to make sure the team has alignment.
A good way to learn from others
Sometimes you don’t understand the purpose of a commmit. This can be a chance to ask questions and make code review a learning opportunity. Or not.