From 70e1a17604a45419b079ab9da5ef69e010b103c6 Mon Sep 17 00:00:00 2001
From: Ash McKenzie <amckenzie@gitlab.com>
Date: Mon, 26 Aug 2019 17:06:15 +1000
Subject: [PATCH] Update some Danger rules to not rely upon CI
Convert a bunch of Danger rules so they can run
locally.
---
danger/database/Dangerfile | 18 ++++++++++---
danger/documentation/Dangerfile | 22 +++++++++-------
danger/duplicate_yarn_dependencies/Dangerfile | 24 +++++++++--------
danger/eslint/Dangerfile | 22 +++++++++-------
danger/frozen_string/Dangerfile | 12 +++++----
danger/gemfile/Dangerfile | 19 +++++++++++---
danger/prettier/Dangerfile | 26 ++++++++++---------
7 files changed, 88 insertions(+), 55 deletions(-)
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index 3550cb7eabf6..5cdad09db6e9 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -1,7 +1,13 @@
# frozen_string_literal: true
-SCHEMA_NOT_UPDATED_MESSAGE = <<~MSG
-**New %<migrations>s added but %<schema>s wasn't updated.**
+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}**
Usually, when adding new %<migrations>s, %<schema>s should be
updated too (unless the migration isn't changing the DB schema
@@ -29,14 +35,18 @@ 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(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'migrations', schema: gitlab.html_link("db/schema.rb"))
+ warn format(format_str, migrations: 'migrations', schema: gitlab_danger.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"))
+ warn format(format_str, migrations: 'Geo migrations', schema: gitlab_danger.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 96c0d9b74782..ad64c3a6c608 100644
--- a/danger/documentation/Dangerfile
+++ b/danger/documentation/Dangerfile
@@ -6,20 +6,22 @@ unless docs_paths_to_review.empty?
message 'This merge request adds or changes files that require a review ' \
'from the Technical Writing team.'
- markdown(<<~MARKDOWN)
-## Documentation review
+ if GitlabDanger.new(helper.gitlab_helper).ci?
+ 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.'
+ unless gitlab.mr_labels.include?('Documentation')
+ warn 'This merge request is missing the ~Documentation label.'
+ end
end
end
diff --git a/danger/duplicate_yarn_dependencies/Dangerfile b/danger/duplicate_yarn_dependencies/Dangerfile
index 25f81ec86a42..492b888d00e5 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,17 +11,19 @@ return if duplicate.empty?
warn 'This merge request has introduced duplicated yarn dependencies.'
-markdown(<<~MARKDOWN)
- ## Duplicate yarn dependencies
+if GitlabDanger.new(helper.gitlab_helper).ci?
+ 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
+ ```
+ node_modules/.bin/yarn-deduplicate --strategy fewer yarn.lock \\
+ && yarn install
+ ```
+ MARKDOWN
+end
diff --git a/danger/eslint/Dangerfile b/danger/eslint/Dangerfile
index 4916cacfd7e3..92830bd77069 100644
--- a/danger/eslint/Dangerfile
+++ b/danger/eslint/Dangerfile
@@ -13,17 +13,19 @@ return if eslint_candidates.empty?
warn 'This merge request changed files with disabled eslint rules. Please consider fixing them.'
-markdown(<<~MARKDOWN)
- ## Disabled eslint rules
+if GitlabDanger.new(helper.gitlab_helper).ci?
+ 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
+ ```
+ node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \\
+ #{eslint_candidates.map { |path| " '#{path}'" }.join(" \\\n")}
+ ```
+ MARKDOWN
+end
diff --git a/danger/frozen_string/Dangerfile b/danger/frozen_string/Dangerfile
index b9687ef6b83b..8d3ac3dee683 100644
--- a/danger/frozen_string/Dangerfile
+++ b/danger/frozen_string/Dangerfile
@@ -16,11 +16,13 @@ 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.'
- markdown(<<~MARKDOWN)
- ## Enable Frozen String Literal
+ if GitlabDanger.new(helper.gitlab_helper).ci?
+ 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
+ * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
+ MARKDOWN
+ end
end
diff --git a/danger/gemfile/Dangerfile b/danger/gemfile/Dangerfile
index dfe64f79d7b4..07c4c07cfe8e 100644
--- a/danger/gemfile/Dangerfile
+++ b/danger/gemfile/Dangerfile
@@ -1,5 +1,9 @@
-GEMFILE_LOCK_NOT_UPDATED_MESSAGE = <<~MSG.freeze
-**%<gemfile>s was updated but %<gemfile_lock>s wasn't updated.**
+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}**
Usually, when %<gemfile>s is updated, you should run
```
@@ -19,5 +23,14 @@ 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"))
+ 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)
end
diff --git a/danger/prettier/Dangerfile b/danger/prettier/Dangerfile
index 37c4b78a213b..0be75db8baab 100644
--- a/danger/prettier/Dangerfile
+++ b/danger/prettier/Dangerfile
@@ -19,21 +19,23 @@ return if unpretty.empty?
warn 'This merge request changed frontend files without pretty printing them.'
-markdown(<<~MARKDOWN)
- ## Pretty print Frontend files
+if GitlabDanger.new(helper.gitlab_helper).ci?
+ 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
+ [on-save]: https://docs.gitlab.com/ee/development/new_fe_guide/style/prettier.html
+ MARKDOWN
+end
--
GitLab