summaryrefslogtreecommitdiff
path: root/danger
diff options
context:
space:
mode:
authorToon Claes <toon@gitlab.com>2019-07-19 17:33:48 +0000
committerNick Thomas <nick@gitlab.com>2019-07-19 17:33:48 +0000
commit34a5f77e770765f278ade00a33ef846e2e1ce3d3 (patch)
tree11f08d25e84c62a2c0f42247f71f76f1ab016012 /danger
parent325360444253cf630ae3bfb6cae47a1e6e612c79 (diff)
downloadgitlab-ce-34a5f77e770765f278ade00a33ef846e2e1ce3d3.tar.gz
Document database review process
See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/6069
Diffstat (limited to 'danger')
-rw-r--r--danger/database/Dangerfile42
-rw-r--r--danger/roulette/Dangerfile15
2 files changed, 28 insertions, 29 deletions
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index 083e95b8da7..3550cb7eabf 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -8,6 +8,21 @@ updated too (unless the migration isn't changing the DB schema
and isn't the most recent one).
MSG
+DB_MESSAGE = <<~MSG
+This merge request requires a database review. To make sure these
+changes are reviewed, take the following steps:
+
+1. Ensure the merge request has ~database and ~"database::review pending" labels.
+ If the merge request modifies database files, Danger will do this for you.
+1. Use the [Database changes checklist](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab/merge_request_templates/Database%20changes.md)
+ template or add the appropriate items to the MR description.
+1. Assign and mention the database reviewer suggested by Reviewer Roulette.
+MSG
+
+DB_FILES_MESSAGE = <<~MSG
+The following files require a review from the Database team:
+MSG
+
non_geo_db_schema_updated = !git.modified_files.grep(%r{\Adb/schema\.rb}).empty?
geo_db_schema_updated = !git.modified_files.grep(%r{\Aee/db/geo/schema\.rb}).empty?
@@ -24,27 +39,18 @@ end
db_paths_to_review = helper.changes_by_category[:database]
-unless db_paths_to_review.empty?
+if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
message 'This merge request adds or changes files that require a ' \
- 'review from the Database team.'
-
- markdown(<<~MARKDOWN.strip)
-## Database Review
-
-The following files require a review from the Database team:
-
-* #{db_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
+ 'review from the [Database team](https://gitlab.com/groups/gl-database/-/group_members).'
-To make sure these changes are reviewed, take the following steps:
+ markdown(DB_MESSAGE)
+ markdown(DB_FILES_MESSAGE + helper.markdown_list(db_paths_to_review)) if db_paths_to_review.any?
-1. Edit your merge request, and add `gl-database` to the list of Group
- approvers.
-1. Mention `@gl-database` in a separate comment, and explain what needs to be
- reviewed by the team. Please don't mention the team until your changes are
- ready for review.
- MARKDOWN
+ database_labels = helper.missing_database_labels(gitlab.mr_labels)
- unless gitlab.mr_labels.include?('database')
- warn 'This merge request is missing the ~database label.'
+ if database_labels.any?
+ gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
+ gitlab.mr_json['iid'],
+ labels: (gitlab.mr_labels + database_labels).join(','))
end
end
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index 6718e218233..19a5076778c 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -55,22 +55,15 @@ def spin_for_category(team, project, category, branch_name)
"| #{helper.label_for_category(category)} | #{reviewer&.markdown_name || NO_REVIEWER} | #{maintainer&.markdown_name || NO_MAINTAINER} |"
end
-def build_list(items)
- list = items.map { |filename| "* `#{filename}`" }.join("\n")
-
- if items.size > 10
- "\n<details>\n\n#{list}\n\n</details>"
- else
- list
- end
-end
-
changes = helper.changes_by_category
# Ignore any files that are known but uncategorized. Prompt for any unknown files
changes.delete(:none)
categories = changes.keys - [:unknown]
+# Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries)
+categories << :database if gitlab.mr_labels.include?('database') && !categories.include?(:database)
+
# Single codebase MRs are reviewed using a slightly different process, so we
# disable the review roulette for such MRs.
# CSS Clean up MRs are reviewed using a slightly different process, so we
@@ -95,5 +88,5 @@ if changes.any? && !gitlab.mr_labels.include?('single codebase') && !gitlab.mr_l
markdown(MESSAGE)
markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty?
- markdown(UNKNOWN_FILES_MESSAGE + build_list(unknown)) unless unknown.empty?
+ markdown(UNKNOWN_FILES_MESSAGE + helper.markdown_list(unknown)) unless unknown.empty?
end