summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAsh McKenzie <amckenzie@gitlab.com>2019-09-06 07:14:30 +0000
committerAsh McKenzie <amckenzie@gitlab.com>2019-09-06 07:14:30 +0000
commitf34e4545ec20953267ee187227116756562c6e8a (patch)
tree32326de09418e404ac74b2b05398bd5449ba8b6c
parent351d72cbed57c5b117e6b2239dffabcedbc45046 (diff)
downloadgitlab-ce-revert-351d72cb.tar.gz
Revert "Merge branch '66596-allow-danger-to-be-run-locally' into 'master'"revert-351d72cb
This reverts merge request !32196
-rw-r--r--Dangerfile23
-rw-r--r--Gemfile1
-rw-r--r--Gemfile.lock33
-rw-r--r--danger/database/Dangerfile18
-rw-r--r--danger/documentation/Dangerfile22
-rw-r--r--danger/duplicate_yarn_dependencies/Dangerfile24
-rw-r--r--danger/eslint/Dangerfile22
-rw-r--r--danger/frozen_string/Dangerfile12
-rw-r--r--danger/gemfile/Dangerfile19
-rw-r--r--danger/prettier/Dangerfile26
-rw-r--r--lib/gitlab/danger/helper.rb6
-rw-r--r--lib/gitlab_danger.rb54
-rw-r--r--lib/tasks/gitlab_danger.rake17
-rw-r--r--spec/lib/gitlab_danger_spec.rb76
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
diff --git a/Gemfile b/Gemfile
index fdb30aeb187..3e3a8c5dde2 100644
--- a/Gemfile
+++ b/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
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