Merge branch '20700-use-danger-gem' into 'master'
Resolve "Use Danger gem" Closes #20700 See merge request gitlab-org/gitlab-ce!19066
This commit is contained in:
commit
ba38931d90
|
@ -0,0 +1 @@
|
|||
Dangerfile gitlab-language=ruby
|
|
@ -348,6 +348,24 @@ 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: {}
|
||||
only:
|
||||
refs:
|
||||
- branches@gitlab-org/gitlab-ce
|
||||
- branches@gitlab-org/gitlab-ee
|
||||
except:
|
||||
variables:
|
||||
- $CI_COMMIT_REF_NAME =~ /^ce-to-ee-.*/
|
||||
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,66 @@
|
|||
# rubocop:disable Style/SignalException
|
||||
|
||||
require 'yaml'
|
||||
|
||||
NO_CHANGELOG_LABELS = %w[backstage QA test].freeze
|
||||
SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html).".freeze
|
||||
MISSING_CHANGELOG_MESSAGE = <<~MSG.freeze
|
||||
**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html).**
|
||||
|
||||
You can create one with:
|
||||
|
||||
```
|
||||
bin/changelog -m %<mr_iid>s
|
||||
```
|
||||
|
||||
If your merge request doesn't warrant a CHANGELOG entry,
|
||||
consider adding any of the %<labels>s labels.
|
||||
#{SEE_DOC}
|
||||
MSG
|
||||
|
||||
def ee?
|
||||
ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md')
|
||||
end
|
||||
|
||||
def ee_changelog?(changelog_path)
|
||||
changelog_path =~ /unreleased-ee/
|
||||
end
|
||||
|
||||
def ce_port_changelog?(changelog_path)
|
||||
ee? && !ee_changelog?(changelog_path)
|
||||
end
|
||||
|
||||
def check_changelog(path)
|
||||
yaml = YAML.safe_load(File.read(path))
|
||||
|
||||
fail "`title` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil?
|
||||
fail "`type` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil?
|
||||
|
||||
if yaml["merge_request"].nil?
|
||||
message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}"
|
||||
elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !ce_port_changelog?(changelog_path)
|
||||
fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
|
||||
end
|
||||
rescue StandardError
|
||||
# YAML could not be parsed, fail the build.
|
||||
fail "#{gitlab.html_link(path)} isn't valid YAML! #{SEE_DOC}"
|
||||
end
|
||||
|
||||
def presented_no_changelog_labels
|
||||
NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ')
|
||||
end
|
||||
|
||||
changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty?
|
||||
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
|
||||
warn format(MISSING_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], labels: presented_no_changelog_labels)
|
||||
end
|
||||
end
|
|
@ -0,0 +1,17 @@
|
|||
# FIXME: git.info_for_file raises the following error
|
||||
# /usr/local/bundle/gems/git-1.4.0/lib/git/lib.rb:956:in `command': (Danger::DSLError)
|
||||
# [!] Invalid `Dangerfile` file:
|
||||
# [!] Invalid `Dangerfile` file: git '--git-dir=/builds/gitlab-org/gitlab-ce/.git' '--work-tree=/builds/gitlab-org/gitlab-ce' cat-file '-t' '' 2>&1:fatal: Not a valid object name
|
||||
# This seems to be the same as https://github.com/danger/danger/issues/535.
|
||||
|
||||
# locale_files_updated = git.modified_files.select { |path| path.start_with?('locale') }
|
||||
# locale_files_updated.each do |locale_file_updated|
|
||||
# git_stats = git.info_for_file(locale_file_updated)
|
||||
# message "Git stats for #{locale_file_updated}: #{git_stats[:insertions]} insertions, #{git_stats[:deletions]} insertions"
|
||||
# end
|
||||
|
||||
if git.lines_of_code > 2_000
|
||||
warn "This merge request is definitely too big (more than #{git.lines_of_code} lines changed), please split it into multiple merge requests."
|
||||
elsif git.lines_of_code > 500
|
||||
warn "This merge request is quite big (more than #{git.lines_of_code} lines changed), please consider splitting it into multiple merge requests."
|
||||
end
|
|
@ -0,0 +1,83 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# All the files/directories that should be reviewed by the DB team.
|
||||
DB_FILES = [
|
||||
'db/',
|
||||
'app/models/project_authorization.rb',
|
||||
'app/services/users/refresh_authorized_projects_service.rb',
|
||||
'lib/gitlab/background_migration.rb',
|
||||
'lib/gitlab/background_migration/',
|
||||
'lib/gitlab/database.rb',
|
||||
'lib/gitlab/database/',
|
||||
'lib/gitlab/github_import.rb',
|
||||
'lib/gitlab/github_import/',
|
||||
'lib/gitlab/sql/',
|
||||
'rubocop/cop/migration',
|
||||
'ee/db/',
|
||||
'ee/lib/gitlab/database/'
|
||||
].freeze
|
||||
|
||||
SCHEMA_NOT_UPDATED_MESSAGE = <<~MSG
|
||||
**New %<migrations>s added but %<schema>s wasn't updated.**
|
||||
|
||||
Usually, when adding new %<migrations>s, %<schema>s should be
|
||||
updated too (unless the migration isn't changing the DB schema
|
||||
and isn't the most recent one).
|
||||
MSG
|
||||
|
||||
def database_paths_requiring_review(files)
|
||||
to_review = []
|
||||
|
||||
files.each do |file|
|
||||
review = DB_FILES.any? do |pattern|
|
||||
file.start_with?(pattern)
|
||||
end
|
||||
|
||||
to_review << file if review
|
||||
end
|
||||
|
||||
to_review
|
||||
end
|
||||
|
||||
all_files = git.added_files + git.modified_files
|
||||
|
||||
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?
|
||||
|
||||
non_geo_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 non_geo_migration_created && !non_geo_db_schema_updated
|
||||
warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'migrations', schema: gitlab.html_link("db/schema.rb"))
|
||||
end
|
||||
|
||||
if geo_migration_created && !geo_db_schema_updated
|
||||
warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'Geo migrations', schema: gitlab.html_link("ee/db/geo/schema.rb"))
|
||||
end
|
||||
|
||||
db_paths_to_review = database_paths_requiring_review(all_files)
|
||||
|
||||
unless db_paths_to_review.empty?
|
||||
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* ")}
|
||||
|
||||
To make sure these changes are reviewed, take the following steps:
|
||||
|
||||
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
|
||||
|
||||
unless gitlab.mr_labels.include?('database')
|
||||
warn 'This merge request is missing the ~database label.'
|
||||
end
|
||||
end
|
|
@ -0,0 +1,24 @@
|
|||
GEMFILE_LOCK_NOT_UPDATED_MESSAGE = <<~MSG.freeze
|
||||
**%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.**
|
||||
|
||||
Usually, when %<gemfile>s is updated, you should run
|
||||
```
|
||||
bundle install && \
|
||||
BUNDLE_GEMFILE=Gemfile.rails5 bundle install
|
||||
```
|
||||
|
||||
or
|
||||
|
||||
```
|
||||
bundle update <the-added-or-updated-gem>
|
||||
```
|
||||
|
||||
and commit the %<gemfile_lock>s changes.
|
||||
MSG
|
||||
|
||||
gemfile_modified = git.modified_files.include?("Gemfile")
|
||||
gemfile_lock_modified = git.modified_files.include?("Gemfile.lock")
|
||||
|
||||
if gemfile_modified && !gemfile_lock_modified
|
||||
warn format(GEMFILE_LOCK_NOT_UPDATED_MESSAGE, gemfile: gitlab.html_link("Gemfile"), gemfile_lock: gitlab.html_link("Gemfile.lock"))
|
||||
end
|
|
@ -0,0 +1,25 @@
|
|||
# rubocop:disable Style/SignalException
|
||||
|
||||
if gitlab.mr_body.size < 5
|
||||
fail "Please provide a proper 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
|
||||
warn "Most of the time, all merge requests should target `master`. Otherwise, please set the relevant `Pick into X.Y` label."
|
||||
end
|
|
@ -0,0 +1,12 @@
|
|||
NO_NEW_SPEC_MESSAGE = <<~MSG.freeze
|
||||
You've made some app changes, but didn't add any tests.
|
||||
That's OK as long as you're refactoring existing code,
|
||||
but please consider adding the ~backstage label in that case.
|
||||
MSG
|
||||
|
||||
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
|
||||
warn NO_NEW_SPEC_MESSAGE, sticky: false
|
||||
end
|
Loading…
Reference in New Issue