Commit 2c8beb6b authored by Mike Jang's avatar Mike Jang

Merge branch 'comments-why-not-what' into 'master'

Update docs to clarify guidance around code comments during review

See merge request gitlab-org/gitlab!52608
parents c50e0466 f2f932a9
...@@ -177,8 +177,11 @@ warrant a comment could be: ...@@ -177,8 +177,11 @@ warrant a comment could be:
Avoid: Avoid:
- Adding comments (referenced above, or TODO items) directly to the source code unless the reviewer requires you to do so. If the comments are added due to an actionable task, - Adding TODO comments (referenced above) directly to the source code unless the reviewer requires
a link to an issue must be included. you to do so. If TODO comments are added due to an actionable task,
[include a link to the relevant issue](code_comments.md).
- Adding comments which only explain what the code is doing. If non-TODO comments are added, they should
[_explain why, not what_](https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/).
- Assigning merge requests with failed tests to maintainers. If the tests are failing and you have to assign, ensure you leave a comment with an explanation. - Assigning merge requests with failed tests to maintainers. If the tests are failing and you have to assign, ensure you leave a comment with an explanation.
- Excessively mentioning maintainers through email or Slack (if the maintainer is reachable - Excessively mentioning maintainers through email or Slack (if the maintainer is reachable
through Slack). If you can't assign a merge request, `@` mentioning a maintainer in a comment is acceptable and in all other cases assigning the merge request is sufficient. through Slack). If you can't assign a merge request, `@` mentioning a maintainer in a comment is acceptable and in all other cases assigning the merge request is sufficient.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment