Merge branch 'revert-351d72cb' into 'master'

Revert "Merge branch '66596-allow-danger-to-be-run-locally' into 'master'"

See merge request gitlab-org/gitlab-ce!32741

Dangerfile

 # frozen_string_literal: true
-
-require_relative 'lib/gitlab_danger'
-
 danger.import_plugin('danger/plugins/helper.rb')
 danger.import_plugin('danger/plugins/roulette.rb')
 
 unless helper.release_automation?
-  GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file|
-    danger.import_dangerfile(path: File.join('danger', file))
-  end
+  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')
+  danger.import_dangerfile(path: 'danger/documentation')
+  danger.import_dangerfile(path: 'danger/frozen_string')
+  danger.import_dangerfile(path: 'danger/commit_messages')
+  danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies')
+  danger.import_dangerfile(path: 'danger/prettier')
+  danger.import_dangerfile(path: 'danger/eslint')
+  danger.import_dangerfile(path: 'danger/roulette')
+  danger.import_dangerfile(path: 'danger/single_codebase')
+  danger.import_dangerfile(path: 'danger/gitlab_ui_wg')
+  danger.import_dangerfile(path: 'danger/ce_ee_vue_templates')
+  danger.import_dangerfile(path: 'danger/only_documentation')
 end

Gemfile

@@ -318,7 +318,6 @@ end
 group :development do
   gem 'foreman', '~> 0.84.0'
   gem 'brakeman', '~> 4.2', require: false
-  gem 'danger', '~> 6.0', require: false
 
   gem 'letter_opener_web', '~> 1.3.4'
   gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false

Gemfile.lock

@@ -140,15 +140,9 @@ GEM
       numerizer (~> 0.1.1)
     chunky_png (1.3.5)
     citrus (3.0.2)
-    claide (1.0.3)
-    claide-plugins (0.9.2)
-      cork
-      nap
-      open4 (~> 1.3)
     coderay (1.1.2)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
-    colored2 (3.1.2)
     commonmarker (0.17.13)
       ruby-enum (~> 0.5)
     concord (0.1.5)
@@ -157,8 +151,6 @@ GEM
     concurrent-ruby (1.1.5)
     connection_pool (2.2.2)
     contracts (0.11.0)
-    cork (0.3.0)
-      colored2 (~> 3.1)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
     crass (1.0.4)
@@ -166,19 +158,6 @@ GEM
     css_parser (1.5.0)
       addressable
     daemons (1.2.6)
-    danger (6.0.9)
-      claide (~> 1.0)
-      claide-plugins (>= 0.9.2)
-      colored2 (~> 3.1)
-      cork (~> 0.1)
-      faraday (~> 0.9)
-      faraday-http-cache (~> 2.0)
-      git (~> 1.5)
-      kramdown (~> 2.0)
-      kramdown-parser-gfm (~> 1.0)
-      no_proxy_fix
-      octokit (~> 4.7)
-      terminal-table (~> 1)
     database_cleaner (1.7.0)
     debug_inspector (0.0.3)
     debugger-ruby_core_source (1.3.8)
@@ -248,8 +227,6 @@ GEM
       railties (>= 3.0.0)
     faraday (0.12.2)
       multipart-post (>= 1.2, < 3)
-    faraday-http-cache (2.0.0)
-      faraday (~> 0.8)
     faraday_middleware (0.12.2)
       faraday (>= 0.7.4, < 1.0)
     faraday_middleware-multi_json (0.0.6)
@@ -331,7 +308,6 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    git (1.5.0)
     gitaly (1.58.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
@@ -502,9 +478,6 @@ GEM
     kgio (2.11.2)
     knapsack (1.17.0)
       rake
-    kramdown (2.1.0)
-    kramdown-parser-gfm (1.1.0)
-      kramdown (~> 2.0)
     kubeclient (4.2.2)
       http (~> 3.0)
       recursive-open-struct (~> 1.0, >= 1.0.4)
@@ -562,12 +535,10 @@ GEM
     mustermann-grape (1.0.0)
       mustermann (~> 1.0.0)
     nakayoshi_fork (0.0.4)
-    nap (1.1.0)
     net-ldap (0.16.0)
     net-ssh (5.2.0)
     netrc (0.11.0)
     nio4r (2.3.1)
-    no_proxy_fix (0.1.2)
     nokogiri (1.10.4)
       mini_portile2 (~> 2.4.0)
     nokogumbo (1.5.0)
@@ -644,7 +615,6 @@ GEM
       addressable (~> 2.5)
       omniauth (~> 1.3)
       openid_connect (~> 1.1)
-    open4 (1.3.4)
     openid_connect (1.1.6)
       activemodel
       attr_required (>= 1.0.0)
@@ -956,8 +926,6 @@ GEM
       ffi
     sysexits (1.2.0)
     temple (0.8.0)
-    terminal-table (1.8.0)
-      unicode-display_width (~> 1.1, >= 1.1.1)
     test-prof (0.2.5)
     text (1.3.1)
     thin (1.7.2)
@@ -1087,7 +1055,6 @@ DEPENDENCIES
   concurrent-ruby (~> 1.1)
   connection_pool (~> 2.0)
   creole (~> 0.5.0)
-  danger (~> 6.0)
   database_cleaner (~> 1.7.0)
   deckar01-task_list (= 2.2.0)
   default_value_for (~> 3.2.0)

danger/database/Dangerfile

 # frozen_string_literal: true
 
-gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
-SCHEMA_NOT_UPDATED_MESSAGE_SHORT = <<~MSG
-New %<migrations>s added but %<schema>s wasn't updated.
-MSG
-
-SCHEMA_NOT_UPDATED_MESSAGE_FULL = <<~MSG
-**#{SCHEMA_NOT_UPDATED_MESSAGE_SHORT}**
+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
@@ -35,18 +29,14 @@ geo_db_schema_updated = !git.modified_files.grep(%r{\Aee/db/geo/schema\.rb}).emp
 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?
 
-format_str = gitlab_danger.ci? ? SCHEMA_NOT_UPDATED_MESSAGE_FULL : SCHEMA_NOT_UPDATED_MESSAGE_SHORT
-
 if non_geo_migration_created && !non_geo_db_schema_updated
-  warn format(format_str, migrations: 'migrations', schema: gitlab_danger.html_link("db/schema.rb"))
+  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(format_str, migrations: 'Geo migrations', schema: gitlab_danger.html_link("ee/db/geo/schema.rb"))
+  warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'Geo migrations', schema: gitlab.html_link("ee/db/geo/schema.rb"))
 end
 
-return unless gitlab_danger.ci?
-
 db_paths_to_review = helper.changes_by_category[:database]
 
 if gitlab.mr_labels.include?('database') || db_paths_to_review.any?

danger/documentation/Dangerfile

@@ -6,22 +6,20 @@ unless docs_paths_to_review.empty?
   message 'This merge request adds or changes files that require a review ' \
     'from the Technical Writing team.'
 
-  if GitlabDanger.new(helper.gitlab_helper).ci?
-    markdown(<<~MARKDOWN)
-      ## Documentation review
+  markdown(<<~MARKDOWN)
+## Documentation review
 
-      The following files require a review from a technical writer:
+The following files require a review from a technical writer:
 
-      * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
+* #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")}
 
-      The review does not need to block merging this merge request. See the:
+The review does not need to block merging this merge request. See the:
 
-      - [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review.
-      - [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review.
-    MARKDOWN
+- [DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages) for the appropriate technical writer for this review.
+- [Documentation workflows](https://docs.gitlab.com/ee/development/documentation/workflow.html) for information on when to assign a merge request for review.
+  MARKDOWN
 
-    unless gitlab.mr_labels.include?('Documentation')
-      warn 'This merge request is missing the ~Documentation label.'
-    end
+  unless gitlab.mr_labels.include?('Documentation')
+    warn 'This merge request is missing the ~Documentation label.'
   end
 end

danger/duplicate_yarn_dependencies/Dangerfile

 # frozen_string_literal: true
 
-return unless helper.all_changed_files.include?('yarn.lock')
+return unless helper.all_changed_files.include? 'yarn.lock'
 
 duplicate = `node_modules/.bin/yarn-deduplicate --list --strategy fewer yarn.lock`
              .split(/$/)
@@ -11,19 +11,17 @@ return if duplicate.empty?
 
 warn 'This merge request has introduced duplicated yarn dependencies.'
 
-if GitlabDanger.new(helper.gitlab_helper).ci?
-  markdown(<<~MARKDOWN)
-    ## Duplicate yarn dependencies
+markdown(<<~MARKDOWN)
+  ## Duplicate yarn dependencies
 
-    The following dependencies should be de-duplicated:
+  The following dependencies should be de-duplicated:
 
-    * #{duplicate.map { |path| "`#{path}`" }.join("\n* ")}
+  * #{duplicate.map { |path| "`#{path}`" }.join("\n* ")}
 
-    Please run the following command and commit the changes to `yarn.lock`:
+  Please run the following command and commit the changes to `yarn.lock`:
 
-    ```
-    node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\
-      && yarn install
-    ```
-  MARKDOWN
-end
+  ```
+  node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\
+    && yarn install
+  ```
+MARKDOWN

danger/eslint/Dangerfile

@@ -13,19 +13,17 @@ return if eslint_candidates.empty?
 
 warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.'
 
-if GitlabDanger.new(helper.gitlab_helper).ci?
-  markdown(<<~MARKDOWN)
-    ## Disabled eslint rules
+markdown(<<~MARKDOWN)
+  ## Disabled eslint rules
 
-    The following files have disabled `eslint` rules. Please consider fixing them:
+  The following files have disabled `eslint` rules. Please consider fixing them:
 
-    * #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")}
+  * #{eslint_candidates.map { |path| "`#{path}`" }.join("\n* ")}
 
-    Run the following command for more details
+  Run the following command for more details
 
-    ```
-    node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\
-    #{eslint_candidates.map { |path| "  '#{path}'" }.join(" \\\n")}
-    ```
-  MARKDOWN
-end
+  ```
+  node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\
+  #{eslint_candidates.map { |path| "  '#{path}'" }.join(" \\\n")}
+  ```
+MARKDOWN

danger/frozen_string/Dangerfile

@@ -16,13 +16,11 @@ if files_to_fix.any?
   warn 'This merge request adds files that do not enforce frozen string literal. ' \
     'See https://gitlab.com/gitlab-org/gitlab-ce/issues/47424 for more information.'
 
-  if GitlabDanger.new(helper.gitlab_helper).ci?
-    markdown(<<~MARKDOWN)
-      ## Enable Frozen String Literal
+  markdown(<<~MARKDOWN)
+    ## Enable Frozen String Literal
 
-      The following files should have `#{MAGIC_COMMENT}` on the first line:
+    The following files should have `#{MAGIC_COMMENT}` on the first line:
 
-      * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
-    MARKDOWN
-  end
+    * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
+  MARKDOWN
 end

danger/gemfile/Dangerfile

-GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT = <<~MSG.freeze
-%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.
-MSG
-
-GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL = <<~MSG.freeze
-**#{GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT}**
+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
 ```
@@ -23,14 +19,5 @@ gemfile_modified = git.modified_files.include?("Gemfile")
 gemfile_lock_modified = git.modified_files.include?("Gemfile.lock")
 
 if gemfile_modified && !gemfile_lock_modified
-  gitlab_danger = GitlabDanger.new(helper.gitlab_helper)
-
-  format_str = gitlab_danger.ci? ? GEMFILE_LOCK_NOT_UPDATED_MESSAGE_FULL : GEMFILE_LOCK_NOT_UPDATED_MESSAGE_SHORT
-
-  message = format(format_str,
-    gemfile: gitlab_danger.html_link("Gemfile"),
-    gemfile_lock: gitlab_danger.html_link("Gemfile.lock")
-  )
-
-  warn(message)
+  warn format(GEMFILE_LOCK_NOT_UPDATED_MESSAGE, gemfile: gitlab.html_link("Gemfile"), gemfile_lock: gitlab.html_link("Gemfile.lock"))
 end

danger/prettier/Dangerfile

@@ -19,23 +19,21 @@ return if unpretty.empty?
 
 warn 'This merge request changed frontend files without pretty printing them.'
 
-if GitlabDanger.new(helper.gitlab_helper).ci?
-  markdown(<<~MARKDOWN)
-    ## Pretty print Frontend files
+markdown(<<~MARKDOWN)
+  ## Pretty print Frontend files
 
-    The following files should have been pretty printed with `prettier`:
+  The following files should have been pretty printed with `prettier`:
 
-    * #{unpretty.map { |path| "`#{path}`" }.join("\n* ")}
+  * #{unpretty.map { |path| "`#{path}`" }.join("\n* ")}
 
-    Please run
+  Please run
 
-    ```
-    node_modules/.bin/prettier --write \\
-    #{unpretty.map { |path| "  '#{path}'" }.join(" \\\n")}
-    ```
+  ```
+  node_modules/.bin/prettier --write \\
+  #{unpretty.map { |path| "  '#{path}'" }.join(" \\\n")}
+  ```
 
-    Also consider auto-formatting [on-save].
+  Also consider auto-formatting [on-save].
 
-    [on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html
-  MARKDOWN
-end
+  [on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html
+MARKDOWN

lib/gitlab/danger/helper.rb

@@ -38,12 +38,8 @@ module Gitlab
         ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md')
       end
 
-      def gitlab_helper
-        gitlab if respond_to?(:gitlab)
-      end
-
       def release_automation?
-        gitlab_helper&.mr_author == RELEASE_TOOLS_BOT
+        gitlab.mr_author == RELEASE_TOOLS_BOT
       end
 
       def project_name

lib/gitlab_danger.rb

-# frozen_string_literal: true
-
-class GitlabDanger
-  LOCAL_RULES ||= %w[
-    changes_size
-    gemfile
-    documentation
-    frozen_string
-    duplicate_yarn_dependencies
-    prettier
-    eslint
-    database
-  ].freeze
-
-  CI_ONLY_RULES ||= %w[
-    metadata
-    changelog
-    specs
-    commit_messages
-    roulette
-    single_codebase
-    gitlab_ui_wg
-    ce_ee_vue_templates
-    only_documentation
-  ].freeze
-
-  MESSAGE_PREFIX = '==>'.freeze
-
-  attr_reader :gitlab_danger_helper
-
-  def initialize(gitlab_danger_helper)
-    @gitlab_danger_helper = gitlab_danger_helper
-  end
-
-  def self.local_warning_message
-    "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}"
-  end
-
-  def self.success_message
-    "#{MESSAGE_PREFIX} No Danger rule violations!"
-  end
-
-  def rule_names
-    ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
-  end
-
-  def html_link(str)
-    self.ci? ? gitlab_danger_helper.html_link(str) : str
-  end
-
-  def ci?
-    !gitlab_danger_helper.nil?
-  end
-end

lib/tasks/gitlab_danger.rake

-desc 'Run local Danger rules'
-task :danger_local do
-  require 'gitlab_danger'
-  require_relative '../../lib/gitlab/popen'
-
-  puts("#{GitlabDanger.local_warning_message}\n")
-
-  # _status will _always_ be 0, regardless of failure or success :(
-  output, _status = Gitlab::Popen.popen(%w{danger dry_run})
-
-  if output.empty?
-    puts(GitlabDanger.success_message)
-  else
-    puts(output)
-    exit(1)
-  end
-end

spec/lib/gitlab_danger_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe GitlabDanger do
-  let(:gitlab_danger_helper) { nil }
-
-  subject { described_class.new(gitlab_danger_helper) }
-
-  describe '.local_warning_message' do
-    it 'returns an informational message with rules that can run' do
-      expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, gemfile, documentation, frozen_string, duplicate_yarn_dependencies, prettier, eslint, database')
-    end
-  end
-
-  describe '.success_message' do
-    it 'returns an informational success message' do
-      expect(described_class.success_message).to eq('==> No Danger rule violations!')
-    end
-  end
-
-  describe '#rule_names' do
-    context 'when running locally' do
-      it 'returns local only rules' do
-        expect(subject.rule_names).to eq(described_class::LOCAL_RULES)
-      end
-    end
-
-    context 'when running under CI' do
-      let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
-      it 'returns all rules' do
-        expect(subject.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES)
-      end
-    end
-  end
-
-  describe '#html_link' do
-    context 'when running locally' do
-      it 'returns the same string' do
-        str = 'something'
-
-        expect(subject.html_link(str)).to eq(str)
-      end
-    end
-
-    context 'when running under CI' do
-      let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
-      it 'returns a HTML link formatted version of the string' do
-        str = 'something'
-        html_formatted_str = %Q{<a href="#{str}">#{str}</a>}
-
-        expect(gitlab_danger_helper).to receive(:html_link).with(str).and_return(html_formatted_str)
-
-        expect(subject.html_link(str)).to eq(html_formatted_str)
-      end
-    end
-  end
-
-  describe '#ci?' do
-    context 'when gitlab_danger_helper is not available' do
-      it 'returns false' do
-        expect(subject.ci?).to be_falsey
-      end
-    end
-
-    context 'when gitlab_danger_helper is available' do
-      let(:gitlab_danger_helper) { double('danger_gitlab_helper') }
-
-      it 'returns true' do
-        expect(subject.ci?).to be_truthy
-      end
-    end
-  end
-end