diff options
author | Ash McKenzie <amckenzie@gitlab.com> | 2019-09-06 07:14:30 +0000 |
---|---|---|
committer | Ash McKenzie <amckenzie@gitlab.com> | 2019-09-06 07:14:30 +0000 |
commit | f34e4545ec20953267ee187227116756562c6e8a (patch) | |
tree | 32326de09418e404ac74b2b05398bd5449ba8b6c | |
parent | 351d72cbed57c5b117e6b2239dffabcedbc45046 (diff) | |
download | gitlab-ce-f34e4545ec20953267ee187227116756562c6e8a.tar.gz |
Revert "Merge branch '66596-allow-danger-to-be-run-locally' into 'master'"revert-351d72cb
This reverts merge request !32196
-rw-r--r-- | Dangerfile | 23 | ||||
-rw-r--r-- | Gemfile | 1 | ||||
-rw-r--r-- | Gemfile.lock | 33 | ||||
-rw-r--r-- | danger/database/Dangerfile | 18 | ||||
-rw-r--r-- | danger/documentation/Dangerfile | 22 | ||||
-rw-r--r-- | danger/duplicate_yarn_dependencies/Dangerfile | 24 | ||||
-rw-r--r-- | danger/eslint/Dangerfile | 22 | ||||
-rw-r--r-- | danger/frozen_string/Dangerfile | 12 | ||||
-rw-r--r-- | danger/gemfile/Dangerfile | 19 | ||||
-rw-r--r-- | danger/prettier/Dangerfile | 26 | ||||
-rw-r--r-- | lib/gitlab/danger/helper.rb | 6 | ||||
-rw-r--r-- | lib/gitlab_danger.rb | 54 | ||||
-rw-r--r-- | lib/tasks/gitlab_danger.rake | 17 | ||||
-rw-r--r-- | spec/lib/gitlab_danger_spec.rb | 76 |
14 files changed, 73 insertions, 280 deletions
diff --git a/Dangerfile b/Dangerfile index 228190cd530..094d55e8652 100644 --- a/Dangerfile +++ b/Dangerfile @@ -1,12 +1,23 @@ # 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 @@ -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 diff --git a/Gemfile.lock b/Gemfile.lock index d787b5c0569..6add217bc32 100644 --- a/Gemfile.lock +++ b/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) diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile index 5cdad09db6e..3550cb7eabf 100644 --- a/danger/database/Dangerfile +++ b/danger/database/Dangerfile @@ -1,13 +1,7 @@ # 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? diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index ad64c3a6c60..96c0d9b7478 100644 --- a/danger/documentation/Dangerfile +++ b/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 diff --git a/danger/duplicate_yarn_dependencies/Dangerfile b/danger/duplicate_yarn_dependencies/Dangerfile index 492b888d00e..25f81ec86a4 100644 --- a/danger/duplicate_yarn_dependencies/Dangerfile +++ b/danger/duplicate_yarn_dependencies/Dangerfile @@ -1,6 +1,6 @@ # 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 diff --git a/danger/eslint/Dangerfile b/danger/eslint/Dangerfile index 92830bd7706..4916cacfd7e 100644 --- a/danger/eslint/Dangerfile +++ b/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 diff --git a/danger/frozen_string/Dangerfile b/danger/frozen_string/Dangerfile index 8d3ac3dee68..b9687ef6b83 100644 --- a/danger/frozen_string/Dangerfile +++ b/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 diff --git a/danger/gemfile/Dangerfile b/danger/gemfile/Dangerfile index 07c4c07cfe8..dfe64f79d7b 100644 --- a/danger/gemfile/Dangerfile +++ b/danger/gemfile/Dangerfile @@ -1,9 +1,5 @@ -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 diff --git a/danger/prettier/Dangerfile b/danger/prettier/Dangerfile index 0be75db8baa..37c4b78a213 100644 --- a/danger/prettier/Dangerfile +++ b/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 diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 702c73e8e4d..17ad07bfc0c 100644 --- a/lib/gitlab/danger/helper.rb +++ b/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 diff --git a/lib/gitlab_danger.rb b/lib/gitlab_danger.rb deleted file mode 100644 index b4768a9546d..00000000000 --- a/lib/gitlab_danger.rb +++ /dev/null @@ -1,54 +0,0 @@ -# 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 diff --git a/lib/tasks/gitlab_danger.rake b/lib/tasks/gitlab_danger.rake deleted file mode 100644 index c2f5843a9a5..00000000000 --- a/lib/tasks/gitlab_danger.rake +++ /dev/null @@ -1,17 +0,0 @@ -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 diff --git a/spec/lib/gitlab_danger_spec.rb b/spec/lib/gitlab_danger_spec.rb deleted file mode 100644 index 623ac20fa7c..00000000000 --- a/spec/lib/gitlab_danger_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -# 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 |