This is another post in our Code Health series. A version of this post originally appeared in Google bathrooms worldwide as a Google Testing on the Toilet episode. You can download a printer-friendly version to display in your office.
By Felipe Sodré and Adam Bender
Have you ever received a code review with a comment like this?
server_is_alive = InitializeServer() Comment:
This doesn’t make any sense. Why don't you use InitializeServerWithAFewExtraSteps() instead? |
|
If so, it probably left you scratching your head. Why is the reviewer asking this question? Are they testing you? Did you make a big mistake? The problem is the comment is vague (and maybe even a bit rude). It also leaves out important context as to what the reviewer is thinking, making it difficult to respond to. Here are a few simple ways to ensure your review comments are effective, helpful, and clear.
- Be kind! People are more receptive to feedback if you assume competence and treat them with respect.
- Focus your comments on the code, not the author. Avoid statements with the word ‘you’ which can give the impression that you are judging the person and not the artifact.
- Explain why you are making the comment. You may be aware of alternatives that are not obvious to the author, or they may be aware of additional constraints.
- Express the tradeoffs your suggestion entails and take a pragmatic approach to working with the author to achieve the right balance.
- Approach your role as a guide, not a test grader. Balance giving direct guidance with leaving some degrees of freedom for the author.
Some comments can be difficult to express concisely. If you have a rewrite in mind, like a subtle API usage or structural change (e.g., split this big change into smaller changes), consider providing an example. If you find you are stuck after more than two rounds of feedback, consider moving to a more direct communication channel like chat or live call to discuss next steps.
When done well, comments can make code reviews more efficient and collaborative:
server_is_alive = InitializeServer() Comment:
InitializeServerWithAFewExtraSteps() seems to achieve the same result but with built-in logging and auditing (see http://short_link_here). Would that be a better option here? |
|
The goal of reviews of any kind is to get the best possible outcome for you and your team. Software Engineering is a team effort, and by helping each other more effectively you will deliver better, together!
References: Code Health: Respectful Reviews == Useful Reviews, Google's guide on Code Review Comments
