Start to use Danger for automating MR reviews
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
d2ea56a870
commit
5679837cd4
|
|
@ -0,0 +1 @@
|
|||
Dangerfile gitlab-language=ruby
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
Loading…
Reference in New Issue