From 2ceee4c89259def0c58d947035eb17afd3b7ceec Mon Sep 17 00:00:00 2001 From: Balasankar 'Balu' C Date: Tue, 16 Aug 2022 22:45:11 +0000 Subject: [PATCH] Use gitlab-dangerfiles gem Use the common features such as changelog checks provided by gitlab-dangerfiles and clean up the file structure for easier maintenance and upkeep with the broader changes across the organization. Signed-off-by: Balasankar "Balu" C --- .gitlab-ci.yml | 4 +- .rubocop.yml | 2 +- Dangerfile | 16 ++-- Gemfile | 4 + Gemfile.lock | 49 +++++++++- danger/metadata/Dangerfile | 1 + danger/reviewers/Dangerfile | 14 +++ .../ruby_upgrade/Dangerfile | 4 +- .../support => danger}/software/Dangerfile | 5 +- {scripts/support => danger}/specs/Dangerfile | 5 +- .../support => danger}/template/Dangerfile | 6 +- gitlab-ci-config/gitlab-com.yml | 13 +-- scripts/support/changelog/Dangerfile | 92 ------------------- scripts/support/metadata/Dangerfile | 32 ------- scripts/support/reviewers/Dangerfile | 15 --- 15 files changed, 94 insertions(+), 168 deletions(-) create mode 100644 danger/metadata/Dangerfile create mode 100644 danger/reviewers/Dangerfile rename {scripts/support => danger}/ruby_upgrade/Dangerfile (63%) rename {scripts/support => danger}/software/Dangerfile (87%) rename {scripts/support => danger}/specs/Dangerfile (65%) rename {scripts/support => danger}/template/Dangerfile (88%) delete mode 100644 scripts/support/changelog/Dangerfile delete mode 100644 scripts/support/metadata/Dangerfile delete mode 100644 scripts/support/reviewers/Dangerfile diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c6f62f435..11d66eb10 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -24,11 +24,9 @@ stages: include: - local: '/gitlab-ci-config/workflow-rules.yml' - local: '/gitlab-ci-config/dev-gitlab-org.yml' - rules: - - if: '$CI_SERVER_HOST == "dev.gitlab.org"' - local: '/gitlab-ci-config/gitlab-com.yml' rules: - - if: '$CI_SERVER_HOST != "dev.gitlab.org"' + - if: '$CI_SERVER_HOST == "gitlab.com"' default: tags: diff --git a/.rubocop.yml b/.rubocop.yml index 8bb6ad1aa..9e4e9b627 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -28,7 +28,7 @@ AllCops: - 'vendor/bundle/**/*' - 'files/gitlab-cookbooks/runit/**/*' - 'scripts/changelog' - - 'scripts/support/changelog/Dangerfile' + - 'danger/changelog/Dangerfile' - 'scripts/security-harness' # No rails in omnibus diff --git a/Dangerfile b/Dangerfile index a2d051b6a..19c577748 100644 --- a/Dangerfile +++ b/Dangerfile @@ -1,7 +1,9 @@ -danger.import_dangerfile(path: 'scripts/support/metadata') -danger.import_dangerfile(path: 'scripts/support/software') -danger.import_dangerfile(path: 'scripts/support/specs') -danger.import_dangerfile(path: 'scripts/support/template') -danger.import_dangerfile(path: 'scripts/support/changelog') -danger.import_dangerfile(path: 'scripts/support/reviewers') -danger.import_dangerfile(path: 'scripts/support/ruby_upgrade') +# frozen_string_literal: true + +require 'gitlab-dangerfiles' + +Gitlab::Dangerfiles.for_project(self) do |gitlab_dangerfiles| + gitlab_dangerfiles.import_plugins + + gitlab_dangerfiles.import_dangerfiles(except: %w[simple_roulette]) +end diff --git a/Gemfile b/Gemfile index 85b6f2cb8..5a5789c14 100644 --- a/Gemfile +++ b/Gemfile @@ -45,6 +45,10 @@ group :packagecloud, optional: true do gem 'thor', '~> 1.2' end +group :danger, optional: true do + gem 'gitlab-dangerfiles', '~> 3.0', require: false +end + group :rubocop do gem 'gitlab-styles', '~> 6.1', require: false end diff --git a/Gemfile.lock b/Gemfile.lock index c2570bbc3..5571d09f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -160,7 +160,13 @@ GEM fauxhai-ng (>= 7.5) rspec (~> 3.0) citrus (3.0.2) + claide (1.1.0) + claide-plugins (0.9.2) + cork + nap + open4 (~> 1.3) coderay (1.1.3) + colored2 (3.1.2) concord (0.1.5) adamantium (~> 0.2.0) equalizer (~> 0.0.9) @@ -170,6 +176,24 @@ GEM mixlib-archive (>= 0.4, < 2.0) corefoundation (0.3.13) ffi (>= 1.15.0) + cork (0.3.0) + colored2 (~> 3.1) + danger (8.6.1) + claide (~> 1.0) + claide-plugins (>= 0.9.2) + colored2 (~> 3.1) + cork (~> 0.1) + faraday (>= 0.9.0, < 2.0) + faraday-http-cache (~> 2.0) + git (~> 1.7) + kramdown (~> 2.3) + kramdown-parser-gfm (~> 1.0) + no_proxy_fix + octokit (~> 4.7) + terminal-table (>= 1, < 4) + danger-gitlab (8.0.0) + danger + gitlab (~> 4.2, >= 4.2.0) diff-lcs (1.3) docile (1.4.0) docker-api (2.0.0) @@ -195,6 +219,8 @@ GEM faraday-em_http (1.0.0) faraday-em_synchrony (1.0.0) faraday-excon (1.1.0) + faraday-http-cache (2.4.1) + faraday (>= 0.8) faraday-net_http (1.0.1) faraday-net_http_persistent (1.2.0) faraday_middleware (1.2.0) @@ -207,9 +233,15 @@ GEM ffi-yajl (2.4.0) libyajl2 (>= 1.2) fuzzyurl (0.9.0) + git (1.11.0) + rchardet (~> 1.8) gitlab (4.17.0) httparty (~> 0.18) terminal-table (~> 1.5, >= 1.5.1) + gitlab-dangerfiles (3.5.0) + danger (>= 8.4.5) + danger-gitlab (>= 8.0.0) + rake gitlab-styles (6.1.0) rubocop (~> 0.91, >= 0.91.1) rubocop-gitlab-security (~> 0.1.1) @@ -248,6 +280,10 @@ GEM json_pure (2.3.1) knapsack (1.17.1) rake + kramdown (2.4.0) + rexml + kramdown-parser-gfm (1.1.0) + kramdown (~> 2.0) libyajl2 (2.1.0) license-acceptance (2.1.13) pastel (~> 0.7) @@ -283,13 +319,18 @@ GEM multi_json (1.15.0) multi_xml (0.6.0) multipart-post (2.1.1) + nap (1.1.0) net-scp (3.0.0) net-ssh (>= 2.6.5, < 7.0.0) net-sftp (3.0.0) net-ssh (>= 5.0.0, < 7.0.0) net-ssh (6.1.0) netrc (0.11.0) + no_proxy_fix (0.1.2) nori (2.6.0) + octokit (4.25.1) + faraday (>= 1, < 3) + sawyer (~> 0.9) ohai (17.9.0) chef-config (>= 14.12, < 18) chef-utils (>= 16.0, < 18) @@ -304,6 +345,7 @@ GEM train-core wmi-lite (~> 1.0) omnibus-ctl (0.3.6) + open4 (1.3.4) package_cloud (0.3.09) highline (~> 2.0.0) json_pure (~> 2.3.0) @@ -337,6 +379,7 @@ GEM rainbow (2.2.2) rake rake (13.0.6) + rchardet (1.8.0) regexp_parser (2.1.1) rest-client (2.1.0) http-accept (>= 1.7.0, < 2.0) @@ -395,6 +438,9 @@ GEM ruby2_keywords (0.0.5) rubyntlm (0.6.3) rubyzip (2.3.2) + sawyer (0.9.2) + addressable (>= 2.3.5) + faraday (>= 0.17.3, < 3) semverse (3.0.0) simplecov (0.21.2) docile (~> 1.1) @@ -508,6 +554,7 @@ DEPENDENCIES docker-api fantaskspec gitlab + gitlab-dangerfiles (~> 3.0) gitlab-styles (~> 6.1) http json @@ -528,4 +575,4 @@ DEPENDENCIES yard BUNDLED WITH - 2.2.33 + 2.3.16 diff --git a/danger/metadata/Dangerfile b/danger/metadata/Dangerfile new file mode 100644 index 000000000..9688d15fc --- /dev/null +++ b/danger/metadata/Dangerfile @@ -0,0 +1 @@ +warn "You may want to add ~group::distribution label to this MR for gitlab-insights" unless helper.has_scoped_label_with_scope?("group") diff --git a/danger/reviewers/Dangerfile b/danger/reviewers/Dangerfile new file mode 100644 index 000000000..3ed820327 --- /dev/null +++ b/danger/reviewers/Dangerfile @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +return if helper.has_scoped_label_with_scope?("workflow") + +REVIEWERS_MESSAGE = <<~MSG +Please add the ~"workflow::ready for review" label once you think the MR is ready to for an initial review. + +Merge requests are handled according to the workflow documented in our [handbook](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/merge_requests.html) and should receive a response within the limit documented in our [First-response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#first-response-slo). + +If you don't receive a response, please mention `@gitlab-org\/distribution`, or one of our [Project Maintainers](https://about.gitlab.com/handbook/engineering/projects/#omnibus-gitlab) +MSG + +# Print maintainers message +message(REVIEWERS_MESSAGE) diff --git a/scripts/support/ruby_upgrade/Dangerfile b/danger/ruby_upgrade/Dangerfile similarity index 63% rename from scripts/support/ruby_upgrade/Dangerfile rename to danger/ruby_upgrade/Dangerfile index 65a4cff03..dc9e4b165 100644 --- a/scripts/support/ruby_upgrade/Dangerfile +++ b/danger/ruby_upgrade/Dangerfile @@ -5,6 +5,6 @@ Please make sure this merge request follows all standards established within the [Ruby upgrade guidelines](https://docs.gitlab.com/ee/development/ruby_upgrade.html). MSG -diff = git.diff_for_file("config/software/ruby.rb") +lines = helper.changed_lines("config/software/ruby.rb") -warn format(RUBY_UPGRADE_MESSAGE) if diff && diff.patch =~ /[+-]+.*default_version/ +warn format(RUBY_UPGRADE_MESSAGE) if lines.any? { |line| line =~ /[+-]+.*default_version/ } diff --git a/scripts/support/software/Dangerfile b/danger/software/Dangerfile similarity index 87% rename from scripts/support/software/Dangerfile rename to danger/software/Dangerfile index 972feb442..7f16fe5c3 100644 --- a/scripts/support/software/Dangerfile +++ b/danger/software/Dangerfile @@ -44,8 +44,7 @@ def library_paths_requiring_review(files) to_review end -all_files = git.added_files + git.modified_files -has_config_changes = !library_paths_requiring_review(all_files).empty? -requires_build_review = (gitlab.mr_labels & NO_BUILD_CHANGE_HAPPENED_LABELS).empty? +has_config_changes = !library_paths_requiring_review(helper.all_changed_files).empty? +requires_build_review = (helper.mr_labels & NO_BUILD_CHANGE_HAPPENED_LABELS).empty? warn format(SOFTWARE_MESSAGE, labels: no_build_change_happened_labels) if has_config_changes && requires_build_review diff --git a/scripts/support/specs/Dangerfile b/danger/specs/Dangerfile similarity index 65% rename from scripts/support/specs/Dangerfile rename to danger/specs/Dangerfile index 6f2f0112e..21e7b50cd 100644 --- a/scripts/support/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -4,7 +4,8 @@ That's OK as long as you're refactoring existing code, but please consider adding the ~"type::maintenance" label in that case. MSG -has_app_changes = !git.modified_files.grep(%r{\A(files|lib)/}).empty? -has_spec_changes = !git.modified_files.grep(/spec/).empty? +all_changed_files = helper.all_changed_files +has_app_changes = all_changed_files.grep(%r{\A(files|lib)/}).any? +has_spec_changes = all_changed_files.grep(/spec/).any? warn NO_NEW_SPEC_MESSAGE, sticky: false if has_app_changes && !has_spec_changes diff --git a/scripts/support/template/Dangerfile b/danger/template/Dangerfile similarity index 88% rename from scripts/support/template/Dangerfile rename to danger/template/Dangerfile index 9d0589e1e..afbb579ea 100644 --- a/scripts/support/template/Dangerfile +++ b/danger/template/Dangerfile @@ -37,9 +37,7 @@ def user_configuration_paths_requiring_review(files) to_review end -all_files = git.added_files + git.modified_files - -configuration_paths_to_review = user_configuration_paths_requiring_review(all_files) +configuration_paths_to_review = user_configuration_paths_requiring_review(helper.all_changed_files) NO_TEMPLATE_CHANGE_MESSAGE = <<~MSG You've made some changes at the locations which contain user facing configuration. @@ -49,4 +47,4 @@ to gitlab.rb.template located in files/gitlab-config-template/gitlab.rb.template Otherwise, please consider adding the ~"type::maintenance" label in that case. MSG -warn NO_TEMPLATE_CHANGE_MESSAGE, sticky: false if !git.modified_files.include?('files/gitlab-config-template/gitlab.rb.template') && !configuration_paths_to_review.empty? +warn NO_TEMPLATE_CHANGE_MESSAGE, sticky: false if !helper.changes.modified.files.include?('files/gitlab-config-template/gitlab.rb.template') && !configuration_paths_to_review.empty? diff --git a/gitlab-ci-config/gitlab-com.yml b/gitlab-ci-config/gitlab-com.yml index 2447b0023..0e73b7ea3 100644 --- a/gitlab-ci-config/gitlab-com.yml +++ b/gitlab-ci-config/gitlab-com.yml @@ -389,16 +389,17 @@ review-docs-cleanup: script: - ./trigger-build.rb docs cleanup +include: + - project: 'gitlab-org/quality/pipeline-common' + file: + - '/ci/danger-review.yml' + danger-review: - image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger stage: check - cache: {} - needs: [] - before_script: [] + variables: + BUNDLE_WITH: "danger" rules: - if: '$PIPELINE_TYPE =~ /_MR_PIPELINE$/' - script: - - danger --fail-on-errors=true Centos 7 knapsack: *prepare_knapsack Centos 8 knapsack: *prepare_knapsack diff --git a/scripts/support/changelog/Dangerfile b/scripts/support/changelog/Dangerfile deleted file mode 100644 index 4c2b1dd10..000000000 --- a/scripts/support/changelog/Dangerfile +++ /dev/null @@ -1,92 +0,0 @@ -# rubocop:disable Style/SignalException - -require 'yaml' - -def check_changelog_trailer(commit) - trailer = commit.message.match(/^(?Changelog):\s*(?.+)$/i) - - return :missing if trailer.nil? || trailer[:category].nil? - - name = trailer[:name] - - unless name == 'Changelog' - self.fail( - "The changelog trailer for commit #{commit.sha} must be `Changelog` (starting with a capital C), not `#{name}`" - ) - - return :invalid - end - - category = trailer[:category] - - return :valid if CATEGORIES.include?(category) - - self.fail( - "Commit #{commit.sha} uses an invalid changelog category: #{category}" - ) - - :invalid -end - -def presented_no_changelog_labels - NO_CHANGELOG_LABELS.map { |label| %(~"#{label}") }.join(', ') -end - -NO_CHANGELOG_LABELS = [ - 'maintenance::pipelines', - 'maintenance::workflow', - 'documentation', - 'ci-build', - 'meta' -].freeze - -CATEGORIES = YAML - .load_file(File.expand_path('../../../.gitlab/changelog_config.yml', __dir__)) - .fetch('categories') - .keys - .freeze - -SEE_DOC = "See [the documentation](https://docs.gitlab.com/ee/development/changelog.html).".freeze - -CHANGELOG_MISSING = <<~MSG.freeze -**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html).** - -To create a changelog, annotate one or more commits with the `Changelog` Git -trailer. If you want to annotate the latest commit, you can do so using `git -commit --amend`. If you want to annotate older or multiple commits, you need to -do so using `git rebase -i`. - -When adding the trailer, you can use the following values: - -- #{CATEGORIES.join("\n- ")} - -For example: - -``` -This is the subject of your commit. - -This would be the body of your commit containing some extra details. - -Changelog: added -``` - -If your merge request doesn't warrant a CHANGELOG entry, consider adding any of -the #{presented_no_changelog_labels} labels. - -#{SEE_DOC} -MSG - -changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? - -if changelog_needed - checked = 0 - - git.commits.each do |commit| - case check_changelog_trailer(commit) - when :valid, :invalid - checked += 1 - end - end - - warn(CHANGELOG_MISSING) if checked.zero? -end diff --git a/scripts/support/metadata/Dangerfile b/scripts/support/metadata/Dangerfile deleted file mode 100644 index d6019877d..000000000 --- a/scripts/support/metadata/Dangerfile +++ /dev/null @@ -1,32 +0,0 @@ -# rubocop:disable Style/SignalException - -WORKTYPE_LABELS = [ - 'type::bug', - 'type::feature', - 'type::maintenance' -].freeze - -fail "Please provide a proper merge request description." if gitlab.mr_body.size < 5 - -fail "Please add labels to this merge request." if gitlab.mr_labels.empty? - -warn "You may want to add ~group::distribution label to this MR for gitlab-insights" unless gitlab.mr_labels.any? { |label| label.start_with?("group::") } - -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." unless gitlab.mr_json["assignee"] - -has_milestone = !gitlab.mr_json["milestone"].nil? - -warn "This merge request does not refer to an existing milestone.", sticky: false unless has_milestone - -has_pick_into_stable_label = gitlab.mr_labels.find { |label| label.start_with?('Pick into') } - -# rubocop:disable Style/IfUnlessModifier -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 - -if (gitlab.mr_labels & WORKTYPE_LABELS).empty? - warn "This merge request is missing any [engineering metrics labels](https://about.gitlab.com/handbook/engineering/metrics/#work-type-classification)." -end -# rubocop:enable Style/IfUnlessModifier -# rubocop:enable Style/SignalException diff --git a/scripts/support/reviewers/Dangerfile b/scripts/support/reviewers/Dangerfile deleted file mode 100644 index c6234bde1..000000000 --- a/scripts/support/reviewers/Dangerfile +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -unless gitlab.mr_labels.any? { |l| l.start_with?('workflow::') } - - REVIEWERS_MESSAGE = <<~MSG - Please add the ~"workflow::ready for review" label once you think the MR is ready to for an initial review. - - Merge requests are handled according to the workflow documented in our [handbook](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/merge_requests.html) and should receive a response within the limit documented in our [First-response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#first-response-slo). - - If you don't receive a response, please mention `@gitlab-org\/distribution`, or one of our [Project Maintainers](https://about.gitlab.com/handbook/engineering/projects/#omnibus-gitlab) - MSG - - # Print maintainers message - message(REVIEWERS_MESSAGE) -end