Commit 53275ad1 authored by Marin Jankovski's avatar Marin Jankovski Committed by Ash McKenzie

What to avoid in code review

parent 7b98bbf6
...@@ -116,8 +116,13 @@ warrant a comment could be: ...@@ -116,8 +116,13 @@ warrant a comment could be:
- Any benchmarking performed to complement the change - Any benchmarking performed to complement the change
- Potentially insecure code - Potentially insecure code
Do not add these comments directly to the source code, unless the Avoid:
reviewer requires you to do so.
- 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,
a link to an issue must be included.
- 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
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.
This This
[saves reviewers time and helps authors catch mistakes earlier](https://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/index.html#__RefHeading__97_174136755). [saves reviewers time and helps authors catch mistakes earlier](https://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/index.html#__RefHeading__97_174136755).
......
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