From 59b4db7f2c9ef94f4bb0e26a12febac97e0611c8 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Mon, 26 Nov 2018 18:04:00 +0000 Subject: Encourage MR author preparation --- doc/development/code_review.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'doc') diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 52710e54e86..e0b72898ed2 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -72,6 +72,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 -- cgit v1.2.3