summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2018-05-21 18:41:21 +0200
committerRémy Coutable <remy@rymai.me>2018-07-11 11:52:01 +0200
commit5679837cd412c2fb7911dcb33c19e89d8a787db0 (patch)
treefe9984ff05042df9ec0fc6de59e85171e337866d
parentd2ea56a87026de92f31bbcfb360748fcf766b835 (diff)
downloadgitlab-ce-5679837cd412c2fb7911dcb33c19e89d8a787db0.tar.gz
Start to use Danger for automating MR reviews
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--.gitattributes1
-rw-r--r--.gitlab-ci.yml14
-rw-r--r--Dangerfile6
-rw-r--r--danger/changelog/Dangerfile39
-rw-r--r--danger/changes_size/Dangerfile9
-rw-r--r--danger/database/Dangerfile16
-rw-r--r--danger/gemfile/Dangerfile15
-rw-r--r--danger/metadata/Dangerfile25
-rw-r--r--danger/specs/Dangerfile13
9 files changed, 138 insertions, 0 deletions
diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 00000000000..f1c41c9bb76
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+Dangerfile gitlab-language=ruby
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 610a5ecba6d..576a96f66f4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -348,6 +348,20 @@ retrieve-tests-metadata:
- wget -O $FLAKY_RSPEC_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$FLAKY_RSPEC_SUITE_REPORT_PATH || rm $FLAKY_RSPEC_SUITE_REPORT_PATH
- '[[ -f $FLAKY_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_SUITE_REPORT_PATH}'
+danger-review:
+ image: registry.gitlab.com/gitlab-org/gitaly/dangercontainer:latest
+ stage: prepare
+ before_script:
+ - source scripts/utils.sh
+ - retry gem install danger --no-ri --no-rdoc
+ cache: {}
+ except:
+ - tags
+ - master
+ script:
+ - git version
+ - danger --fail-on-errors=true
+
update-tests-metadata:
<<: *tests-metadata-state
<<: *only-canonical-masters
diff --git a/Dangerfile b/Dangerfile
new file mode 100644
index 00000000000..84b72673c50
--- /dev/null
+++ b/Dangerfile
@@ -0,0 +1,6 @@
+danger.import_dangerfile(path: 'danger/metadata')
+danger.import_dangerfile(path: 'danger/changes_size')
+danger.import_dangerfile(path: 'danger/changelog')
+danger.import_dangerfile(path: 'danger/specs')
+danger.import_dangerfile(path: 'danger/gemfile')
+danger.import_dangerfile(path: 'danger/database')
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
new file mode 100644
index 00000000000..773ffa15a0f
--- /dev/null
+++ b/danger/changelog/Dangerfile
@@ -0,0 +1,39 @@
+# rubocop:disable Style/SignalException
+
+require 'yaml'
+
+def check_changelog(path)
+ yaml = YAML.safe_load(File.read(path))
+
+ fail "`title` should be set, in #{gitlab.html_link(path)}." if yaml["title"].nil?
+ fail "`type` should be set, in #{gitlab.html_link(path)}." if yaml["type"].nil?
+
+ if yaml["merge_request"].nil?
+ message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}."
+ elsif yaml["merge_request"] != gitlab.mr_json["iid"]
+ fail "Merge request IID was not set to #{gitlab.mr_json["iid"]}!"
+ end
+rescue StandardError
+ # YAML could not be parsed, fail the build.
+ fail "#{gitlab.html_link(path)} isn't valid YAML!"
+end
+
+changelog_needed = !gitlab.mr_labels.include?("backstage")
+changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} }
+
+if git.modified_files.include?("CHANGELOG.md")
+ fail "CHANGELOG.md was edited. Please remove the additions and create an entry with `bin/changelog -m #{gitlab.mr_json["iid"]}` instead."
+end
+
+if changelog_needed
+ if changelog_found
+ check_changelog(path)
+ else
+ msg = [
+ "This merge request is missing a CHANGELOG entry, you can create one with `bin/changelog -m #{gitlab.mr_json["iid"]}`.",
+ "If your merge request doesn't warrant a CHANGELOG entry, consider adding the ~backstage label."
+ ]
+
+ warn msg.join(" ")
+ end
+end
diff --git a/danger/changes_size/Dangerfile b/danger/changes_size/Dangerfile
new file mode 100644
index 00000000000..4471e1926c9
--- /dev/null
+++ b/danger/changes_size/Dangerfile
@@ -0,0 +1,9 @@
+# rubocop:disable Style/SignalException
+
+if git.lines_of_code > 500
+ warn "This merge request is quite big, please consider splitting it into multiple merge requests."
+end
+
+if git.lines_of_code > 5_000
+ fail "This merge request is definitely too big, please split it into multiple merge requests."
+end
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
new file mode 100644
index 00000000000..136dcef7972
--- /dev/null
+++ b/danger/database/Dangerfile
@@ -0,0 +1,16 @@
+# rubocop:disable Style/SignalException
+
+db_schema_updated = !git.modified_files.grep(%r{\A(ee/)?(db/(geo/)?(post_)?migrate)/}).empty?
+migration_created = !git.added_files.grep(%r{\A(db/(post_)?migrate)/}).empty?
+geo_migration_created = !git.added_files.grep(%r{\Aee/(db/geo/(post_)?migrate)/}).empty?
+
+if (migration_created || geo_migration_created) && !db_schema_updated
+ msg = ["New migrations were added but #{gitlab.html_link("db/schema.rb")}"]
+ msg << "(nor #{gitlab.html_link("ee/db/geo/schema.rb")})" if geo_migration_created
+ msg << "wasn't. Usually, when adding new migrations, #{gitlab.html_link("db/schema.rb")}"
+ msg << "(and #{gitlab.html_link("ee/db/geo/schema.rb")})" if geo_migration_created
+ msg << "should be updated too (unless your migrations are data migrations and your"
+ msg << "migration isn't the most recent one)."
+
+ warn msg.join(" ")
+end
diff --git a/danger/gemfile/Dangerfile b/danger/gemfile/Dangerfile
new file mode 100644
index 00000000000..3d38959a9d0
--- /dev/null
+++ b/danger/gemfile/Dangerfile
@@ -0,0 +1,15 @@
+# rubocop:disable Style/SignalException
+
+gemfile_modified = git.modified_files.include?("Gemfile")
+gemfile_lock_modified = git.modified_files.include?("Gemfile.lock")
+
+if gemfile_modified && !gemfile_lock_modified
+ msg = [
+ "#{gitlab.html_link("Gemfile")} was edited but #{gitlab.html_link("Gemfile.lock")} wasn't.",
+ "Usually, when #{gitlab.html_link("Gemfile")} is updated, you should run",
+ "`bundle install && BUNDLE_GEMFILE=Gemfile.rails5 bundle install` or",
+ "`bundle update <the-added-or-updated-gem>` and commit the #{gitlab.html_link("Gemfile.lock")} changes."
+ ]
+
+ warn msg.join(" ")
+end
diff --git a/danger/metadata/Dangerfile b/danger/metadata/Dangerfile
new file mode 100644
index 00000000000..033a84d6ecd
--- /dev/null
+++ b/danger/metadata/Dangerfile
@@ -0,0 +1,25 @@
+# rubocop:disable Style/SignalException
+
+if gitlab.mr_body.size < 5
+ fail "Please provide a merge request description."
+end
+
+if gitlab.mr_labels.empty?
+ fail "Please add labels to this merge request."
+end
+
+unless gitlab.mr_json["assignee"]
+ warn "This merge request does not have any assignee yet. Setting an assignee clarifies who needs to take action on the merge request at any given time."
+end
+
+has_milestone = !gitlab.mr_json["milestone"].nil?
+
+unless has_milestone
+ warn "This merge request does not refer to an existing milestone.", sticky: false
+end
+
+has_pick_into_stable_label = gitlab.mr_labels.find { |label| label.start_with?('Pick into') }
+
+if gitlab.branch_for_base != "master" && !has_pick_into_stable_label
+ fail "All merge requests should target `master`. Otherwise, please set the relevant `Pick into X.Y` label."
+end
diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile
new file mode 100644
index 00000000000..88e64c57a4b
--- /dev/null
+++ b/danger/specs/Dangerfile
@@ -0,0 +1,13 @@
+# rubocop:disable Style/SignalException
+
+has_app_changes = !git.modified_files.grep(%r{\A(ee/)?(app|lib|db/(geo/)?(post_)?migrate)/}).empty?
+has_spec_changes = !git.modified_files.grep(/spec/).empty?
+
+if has_app_changes && !has_spec_changes
+ msg = [
+ "You've made some app changes, but didn't add any tests.",
+ "That's OK as long as you're refactoring existing code (please consider adding the ~backstage label in that case)."
+ ]
+
+ warn msg.join(" "), sticky: false
+end