diff options
| author | Marin Jankovski <marin@gitlab.com> | 2019-09-03 14:47:37 +0000 |
|---|---|---|
| committer | Ash McKenzie <amckenzie@gitlab.com> | 2019-09-04 10:57:10 +1000 |
| commit | 53275ad1a2d168f946b563dcee97610a358ec26d (patch) | |
| tree | 9fa36053261ccc56fb68cb56bb2662753766d883 | |
| parent | 7b98bbf6095d41e427f726a0f9d56a19a0159f6a (diff) | |
| download | gitlab-ce-mj/code-review-avoid.tar.gz | |
What to avoid in code reviewmj/code-review-avoid
| -rw-r--r-- | doc/development/code_review.md | 9 |
1 files changed, 7 insertions, 2 deletions
diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 80890d96159..bcfc0734c06 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -116,8 +116,13 @@ warrant a comment could be: - Any benchmarking performed to complement the change - Potentially insecure code -Do not add these comments directly to the source code, unless the -reviewer requires you to do so. +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, +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 [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). |