diff options
| -rw-r--r-- | doc/development/code_review.md | 25 | ||||
| -rw-r--r-- | locale/gitlab.pot | 3 | ||||
| -rwxr-xr-x[-rw-r--r--] | vendor/gitignore/C++.gitignore | 0 | ||||
| -rwxr-xr-x[-rw-r--r--] | vendor/gitignore/Java.gitignore | 0 |
4 files changed, 22 insertions, 6 deletions
diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 0afa44a5491..ce9f794c1a3 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -87,9 +87,9 @@ Before assigning a merge request to a maintainer for approval and merge, they should be confident that it actually solves the problem it was meant to solve, that it does so in the most appropriate way, that it satisfies all requirements, and that there are no remaining bugs, logical problems, uncovered edge cases, -or known vulnerabilities. The merge request should also have a completed task -list in its description and a passing CI pipeline to avoid unnecessary back and -forth. +or known vulnerabilities. The best way to do this, and to avoid unnecessary +back-and-forth with reviewers, is to perform a self-review of your own merge +request, following the [Code Review](#reviewing-code) guidelines. To reach the required level of confidence in their solution, an author is expected to involve other people in the investigation and implementation processes as @@ -127,6 +127,17 @@ through Slack). If you can't assign a merge request, `@` mentioning a maintainer 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 reviewer + +[Review the merge request](#reviewing-code) thoroughly. When you are confident +that it meets all requirements, you should: + +- Click the Approve button. +- Advise the author their merge request has been reviewed and approved. +- Assign the merge request to a maintainer. [Reviewer roulette](#reviewer-roulette) +should have made a suggestion, but feel free to override if someone else is a +better choice. + ### The responsibility of the maintainer Maintainers are responsible for the overall health, quality, and consistency of @@ -284,6 +295,14 @@ experience, refactors the existing code). Then: - Assign the merge request to the author if changes are required following your review. - Set the milestone before merging a merge request. +- Ensure the target branch is not too far behind master. If +[master is red](https://about.gitlab.com/handbook/engineering/workflow/#broken-master), +it should be no more than 100 commits behind. +- Consider warnings and errors from danger bot, codequality, and other reports. +Unless a strong case can be made for the violation, these should be resolved +before merge. +- Ensure a passing CI pipeline or if [master is broken](https://about.gitlab.com/handbook/engineering/workflow/#broken-master), post a comment mentioning the failure happens in master with a +link to the ~"master:broken" issue. - Avoid accepting a merge request before the job succeeds. Of course, "Merge When Pipeline Succeeds" (MWPS) is fine. - If you set the MR to "Merge When Pipeline Succeeds", you should take over diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5aa9e795c64..97213a026f2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1321,9 +1321,6 @@ msgstr "" msgid "Alternate support URL for help page and help dropdown" msgstr "" -msgid "Alternatively, you can use a %{personal_access_token_link}. When you create your Personal Access Token, you will need to select the <code>repo</code> scope, so we can display a list of your public and private repositories which are available to connect." -msgstr "" - msgid "Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication" msgstr "" diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100644..100755 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100644..100755 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |