Tips for effective, empathetic code reviews

Summary

Code reviews are a key necessity in the development process but are often a source of anxiety and tension when they should be a opportunity to learn and grow. The whole point of code reviews is to extend the life of our code. Reducing pain points and making code reviews as easy as possible for everyone involved is the goal.

Understand what is important to your company and team

For code reviews to be effective everyone involved should have a good understanding of what is to be expected of the code. Does your company have coding standards? Does or team have a style guide? If a team member asks for a code review are they expecting you to understand the code and provide design feedback? Are they expecting you to make sure the class names match the file names and each conditional has curly braces? Everyone involved in the process should know these answers before a single line of code is written.

Use linters and other analysis tools

Linters and analysis tools are a great way of programmatically enforcing company and team standards and styles. The tools usually have a way of automatically formatting and modifying code as well as simply reporting issues. Using these tools allows reviewers to focus on actual issues such as design, performance, and defects.

Start high-level

Code reviews can last multiple rounds; to avoid overwhelming your coworker with long lists of review notes start with high level items first. Often times when taking care of the high level items the smaller nitpicky items get resolved automatically. This also prevents the reviewer from spending unnecessary time writing soon to be irrelevant notes.

Narrow the scope and limit your time

Focus on reviewing 200 lines of code or less in under 20 minutes. By requiring developers to limit the size of checkins we can help mitigate larger defects sooner and turn around review modifications quicker. It is much easier to ask a developer to remove dependencies from a couple classes than it is to introduce dependency injection into a commit that has 3 weeks worth of work put into it. The smaller the code base a reviewer has to review the easier it is for them understand the code, make meaningful suggestions, and quicker the author can perform the rework.

Ask questions

Do not be afraid to ask questions. Be the devil's advocate. If something seems "off" about some code, or you're seeing signs of what may potentially be a problem, then reach out. Ask the author to clarify and if you feel it is necessary, provide some historical or personal anecdote to ease tensions.

Source your claims, provide examples, mark as optional

As a reviewer you need to take into consideration the impact your notes have on the codebase, project timeline, and the authors time. If you're going to claim that comparing uppercase strings is faster than comparing lower case strings (which is true) then you should have sources to backup your statement. Before you click that "needs work" button though, consider the impact of your review. While string comparison can be optimized, how much data are you dealing with? Will this code only be comparing small strings infrequently? Is it really worth the company's and developer's time to go back and change their code?

Don't be afraid to mark notes as optional. Use this as a teaching experience.

"Looks good. One optional fix/something to think about in the future: comparing uppercase strings is marginally better than lowercase [source]. I'm not suggesting you make any changes now, but in the future, go for it!"

This turns into a win-win situation as the reviewer isn't spending unnecessary time writing irrelevant notes and the author has an opportunity to learn.

Keep it short, keep it managable

If you find yourself reading through the list of notes and you feel like you've made quite a collection then the author will most likely feel it even more. Give the author room to breath, don't make it seem like a chore. Often times when taking care of high level items the smaller, nitpicky stuff gets taken care of automatically.

Be positive, be polite, be thoughtful

To foster a culture of open education and striving for better code bases everyone needs to be on board. Demeaning or aggressive comments lead to a breakdown in processes and puts people in defensive positions instead of open and accepting.

The purpose of code reviews should be to fix problems or potential problems, to strengthen the codebase and educate your peers to make better decisions in the future. The more open and feely the code reviews are the more people will be willing to be involved and committed.

Be understanding

While the point of code reviews is to extend the life of a code base you cannot simply overlook the situations which guided the developers decisions. If code looks thrown together you should carefully consider the "why". Does the developer usually write sloppy code, are they overwhelmed right now with too much work or perhaps personal life? Empathy and understanding go a long way to building better relationships and stronger teams. Good intentions don't often translate well in review notes. Don't be afraid to get out of your chair or pick up a phone and actually talk with the author.

Just say "looks good"

Sometimes it's okay to say ‘no issues here, good job'. If you feel like you're hunting for an issue to report and coming up with nothing you're more than likely being nitpicky and fire starting. If you see one style issue, let it go (you should have tools to take care of it anyway). Everything looks good except a method has too many parameters and would be better off accepting a class? Mark as optional and tell the developer "looks good".

Call out good code

If you want to take part in giving people reasons to continue to grow and do better work, then give them more reasons. Don't just stop at "looks good", call out particularly good code.

"Looks good, and nice job with open/closed principle on that new class you added"
"Looks good, and thank some god someone is adding tests to this code base"

Not only does this encourage the person to continue, it helps you build rapport and strengthen your relationships.

Conclusion

The old adage "treat others how you would want to be treated" may apply in code reviews (masochists please ignore). The point of this process is to extend the life of your code base, increase ROI, and educate your coworkers. Don't forget you're dealing with people not machines. Depending on how you decide to go about reviewing code you can either build better relationships or tear them down and reinforce defenses. Use the process as a chance to learn, educate, and build a better product, team and company.