diff options
Diffstat (limited to 'doc')
-rw-r--r-- | doc/development/code_review.md | 17 |
1 files changed, 17 insertions, 0 deletions
diff --git a/doc/development/code_review.md b/doc/development/code_review.md index df2cb30c5d6..7f3d23b5fb0 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -85,6 +85,23 @@ If an author is unsure if a merge request needs a domain expert's opinion, that' usually a pretty good sign that it does, since without it the required level of confidence in their solution will not have been reached. +Before the review, the author is requested to submit comments on the merge +request diff alerting the reviewer to anything important as well as for anything +that demands further explanation or attention. Examples of content that may +warrant a comment could be: + +- The addition of a linting rule (Rubocop, JS etc) +- The addition of a library (Ruby gem, JS lib etc) +- Where not obvious, a link to the parent class or method +- 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. + +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). + ### The responsibility of the maintainer Maintainers are responsible for the overall health, quality, and consistency of |