...
If you had to think about the design of your code, explain your design in prose in a comment no later than when you submit the code for review.
Write as if the audience was yourself prior to you writing the code.
When exploring / analyzing existing code, if you notice that you need to think hard to understand it, then make a PR to add comments with your new understanding. Also fix typos and grammar mistakes that impair the reading, as you find them.
Do so even if it's not your own code.When there are questions during a code review, add comments to explain the outcome of the review:
if the reviewer had questions, the next reader will too.
Engineering standards (checked during reviews)
Do’s:
Use full sentences. Capital letter at begin, period at the end, with at least a subject and a verb.
In a struct, API or protobuf definition, there should be more comments than code.
The comments explain the purpose of the fields, as well as their lifecycle: what code initializes them, what code accesses them and why, when fields become obsolete, etc.
This rule becomes a “must” if the struct/API/protobuf is part of a public/exported API.
Throughout a function body, use comments to separate and summarize different “phases” of the processing in the control flow.
Write these comments such that they would be good explanatory comments if the corresponding part of the function body was extracted into a separate function.
Make an honest attempt at keeping your comments grammatical.
Conversely, assume any reviewer suggestions about grammar is done on a neutral and non-accusatory tone. As a reviewer, prefix grammar-related comments as “nit:”.During reviews, point out when adding a (missing) comment would have helped your understanding.
Give praise for well-written comments.
Don’t:
Write comments that read the code underneath “out loud”, for example:
“this line increments the variable x” for
x++.“now we iterate over all the strings” for
for i := range strs
When reviewing a PR, put too much emphasis on comment grammar.
Grammar remains prefixed by “nits:” in reviews.
Updating comments over time
...
When you discover something valuable that should be explained in a comment, but is not yet explained, consider adding/extending a comment with your new knowledge. The next reader will thank you.
...
If you find a mistake in a comment, it can be a good use of your time to fix it.
If a comment is factually incorrect, then you should consider it as an outright bug and either attempt to fix it immediately or file an issue.
If it contains a grammatical or spelling error that makes the reading particularly difficult, it would be valuable to the team and your future self to fix it too.
...
Beware of “cosmetic” improvements, e.g. changing one word for another, or fixing “simple” typos that don’t hurt reading. The time spent on creating a commit and a PR and getting it reviewed might not be worth the marginal value of the improvement, unless you can combine the improvement with a larger project.
...
Updating comments over time
When you discover something valuable that should be explained in a comment, but is not yet explained, consider adding/extending a comment with your new knowledge. The next reader will thank you.
If you find a mistake in a comment, it can be a good use of your time to fix it.
If a comment is factually incorrect, then you should consider it as an outright bug and either attempt to fix it immediately or file an issue.
If it contains a grammatical or spelling error that makes the reading particularly difficult, it would be valuable to the team and your future self to fix it too.
Beware of “cosmetic” improvements, e.g. changing one word for another, or fixing “simple” typos that don’t hurt reading. The time spent on creating a commit and a PR and getting it reviewed might not be worth the marginal value of the improvement, unless you can combine the improvement with a larger project.
Also consider the marginal impact of aesthetic changes on automation and the output of
git blame, as discussed here.
Engineering standards (checked during reviews)
Do’s:
Use full sentences. Capital letter at begin, period at the end, with at least a subject and a verb.
In a struct, API or protobuf definition, there should be more comments than code.
The comments explain the purpose of the fields, as well as their lifecycle: what code initializes them, what code accesses them and why, when fields become obsolete, etc.
This rule becomes a “must” if the struct/API/protobuf is part of a public/exported API.
Throughout a function body, use comments to separate and summarize different “phases” of the processing in the control flow.
Write these comments such that they would be good explanatory comments if the corresponding part of the function body was extracted into a separate function.
Make an honest attempt at keeping your comments grammatical.
Conversely, assume any reviewer suggestions about grammar is done on a neutral and non-accusatory tone. As a reviewer, prefix grammar-related comments as “nit:”.During reviews, point out when adding a (missing) comment would have helped your understanding.
Give praise for well-written comments.
Don’t:
Write comments that read the code underneath “out loud”, for example:
“this line increments the variable x” for
x++.“now we iterate over all the strings” for
for i := range strs
When reviewing a PR, put too much emphasis on comment grammar.
Grammar remains prefixed by “nits:” in reviews.
Examples
Top-level design comments
...