“If we can’t understand each other, we’ll never understand each other’s code,” said Accolade Senior Engineering Manager Ben Horst.
According to Horst, empathy is a key component of code reviews at the healthtech company. On his team, reviewers approach each review with the assumption that the developer had a reason for the way they wrote something. This attitude helps maintain a positive team culture and reduces the opportunity for misunderstandings.
Accolade produces a suite of platforms for employers that help their staff (over 1.5 million employees to date) manage their health benefits and find the best care management. To maintain the code powering the platforms, Horst said his team requires that code be approved by two people before it’s committed. His team also utilizes static analysis with specific linting rules to keep code style uniform.
But it’s not just the line-by-line details of the reviews that lead to the success of his team’s code, Horst said; it’s the effectiveness of human-to-human interactions.
What best practices does your team follow when doing code reviews?
Our first best practice is that two of your coworkers must approve your code before committing to our test environment. This check helps keep us honest, allows the test environment to run smoothly and keeps us in touch with what our teammates are doing.
Second, our mantra is, “Am I willing to support this code?” We have an on-call rotation that includes everybody on the team.
When we go on call, it’s our job to keep the service up and functional. If the code we approved breaks, will we be happy because we already know how it works? Or upset? Code reviewers should push for code they’re comfortable with and proud of.
Think about how you’d want to hear the feedback you’re giving.”
When there’s a difference in opinions about how to write certain code, how does your team handle it?
We try to keep opinions out of it as much as possible.
Code style, for example, should not be a matter of opinion. We have static analysis including quite a few linting rules. This prevents some errors, but the main point is to keep code consistent. Readability is often a function of consistency, so we strive to codify our consistency in our static analysis tools. If people disagree on whitespace or ternary formatting or where brackets belong, we hash that argument out. We commit to a change that the linter enforces automatically and we agree to move on. No argument should repeat itself often.
What advice do you have for fostering a positive code review culture?
As an author, put yourself in the reviewer’s shoes. What do you want to see in a code review description? What context do you need? Make sure to write those details out for reviewers. Review your own differences first. If something isn’t self-explanatory, consider making some comments. Consider whether those comments are worth putting directly into the code (they probably are).
As a reviewer, put yourself in the author’s shoes. Try to understand what constraints they’re under to get this code into the product. Think about how you’d want to hear the feedback you’re giving. Choose the battles that will yield the most value for the product and forget the rest.
When things get dicey and you’re not sure how to respond to a particular review, consider sitting down with the person to have them walk you through it instead of typing out a letter on the review. As a reviewer, walk the coder through constructive suggestions on a whiteboard or wherever is most comfortable.
If we can’t understand each other, we’ll never understand each other’s code. The code is for a computer, but it comes from a human.
