diff options
author | Rémy Coutable <remy@rymai.me> | 2018-05-21 18:41:21 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-07-11 11:52:01 +0200 |
commit | 5679837cd412c2fb7911dcb33c19e89d8a787db0 (patch) | |
tree | fe9984ff05042df9ec0fc6de59e85171e337866d | |
parent | d2ea56a87026de92f31bbcfb360748fcf766b835 (diff) | |
download | gitlab-ce-5679837cd412c2fb7911dcb33c19e89d8a787db0.tar.gz |
Start to use Danger for automating MR reviews
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | .gitattributes | 1 | ||||
-rw-r--r-- | .gitlab-ci.yml | 14 | ||||
-rw-r--r-- | Dangerfile | 6 | ||||
-rw-r--r-- | danger/changelog/Dangerfile | 39 | ||||
-rw-r--r-- | danger/changes_size/Dangerfile | 9 | ||||
-rw-r--r-- | danger/database/Dangerfile | 16 | ||||
-rw-r--r-- | danger/gemfile/Dangerfile | 15 | ||||
-rw-r--r-- | danger/metadata/Dangerfile | 25 | ||||
-rw-r--r-- | danger/specs/Dangerfile | 13 |
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 |