summaryrefslogtreecommitdiff
path: root/danger
diff options
context:
space:
mode:
Diffstat (limited to 'danger')
-rw-r--r--danger/frozen_string/Dangerfile29
-rw-r--r--danger/product_intelligence/Dangerfile (renamed from danger/product_analytics/Dangerfile)20
-rw-r--r--danger/roulette/Dangerfile7
-rw-r--r--danger/specs/Dangerfile17
4 files changed, 53 insertions, 20 deletions
diff --git a/danger/frozen_string/Dangerfile b/danger/frozen_string/Dangerfile
index 7c4c4f2b17c..bc598b16463 100644
--- a/danger/frozen_string/Dangerfile
+++ b/danger/frozen_string/Dangerfile
@@ -1,15 +1,34 @@
# frozen_string_literal: true
FILE_EXTENSION = ".rb"
-MAGIC_COMMENT = "# frozen_string_literal: true"
+FROZEN_STRING_MAGIC_COMMENT = "# frozen_string_literal: true"
+SHEBANG_COMMENT = "#!"
def get_files_with_no_magic_comment(files)
- files.select do |file|
- file.end_with?(FILE_EXTENSION) &&
- !File.open(file, &:gets)&.start_with?(MAGIC_COMMENT)
+ files.select do |path|
+ path.end_with?(FILE_EXTENSION) &&
+ !file_has_frozen_string_magic_comment?(path)
end
end
+def file_has_frozen_string_magic_comment?(path)
+ File.open(path) do |file|
+ first_line = file.gets
+
+ line_has_frozen_string_magic_comment?(first_line) ||
+ (line_has_shebang?(first_line) &&
+ line_has_frozen_string_magic_comment?(file.gets))
+ end
+end
+
+def line_has_frozen_string_magic_comment?(line)
+ line&.start_with?(FROZEN_STRING_MAGIC_COMMENT)
+end
+
+def line_has_shebang?(line)
+ line&.start_with?(SHEBANG_COMMENT)
+end
+
files_to_fix = get_files_with_no_magic_comment(git.added_files)
if files_to_fix.any?
@@ -20,7 +39,7 @@ if files_to_fix.any?
markdown(<<~MARKDOWN)
## Enable Frozen String Literal
- The following files should have `#{MAGIC_COMMENT}` on the first line:
+ The following files should have `#{FROZEN_STRING_MAGIC_COMMENT}` on the first line:
* #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")}
MARKDOWN
diff --git a/danger/product_analytics/Dangerfile b/danger/product_intelligence/Dangerfile
index b2144e7f034..ec432544977 100644
--- a/danger/product_analytics/Dangerfile
+++ b/danger/product_intelligence/Dangerfile
@@ -1,8 +1,8 @@
# frozen_string_literal: true
-PRODUCT_ANALYTICS_CHANGED_FILES_MESSAGE = <<~MSG
-For the following files, a review from the [Data team and Product Analytics team](https://gitlab.com/groups/gitlab-org/growth/product_analytics/engineers/-/group_members?with_inherited_permissions=exclude) is recommended
-Please check the ~"product analytics" [guide](https://docs.gitlab.com/ee/development/product_analytics/usage_ping.html) and reach out to %<product_analytics_engineers_group>s group for a review.
+CHANGED_FILES_MESSAGE = <<~MSG
+For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended
+Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/product_analytics/usage_ping.html) and reach out to %<engineers_group>s group for a review.
%<changed_files>s
@@ -14,7 +14,7 @@ UPDATE_METRICS_DEFINITIONS_MESSAGE = <<~MSG
MSG
-PRODUCT_ANALYTICS_ENGINEERS_GROUP = '@gitlab-org/growth/product_analytics/engineers'
+ENGINEERS_GROUP = '@gitlab-org/growth/product-intelligence/engineers'
tracking_files = [
'lib/gitlab/tracking.rb',
@@ -33,16 +33,16 @@ changed_files = (usage_data_changed_files + snowplow_events_changed_files)
if changed_files.any?
mention = if helper.draft_mr?
- "`#{PRODUCT_ANALYTICS_ENGINEERS_GROUP}`"
+ "`#{ENGINEERS_GROUP}`"
else
- PRODUCT_ANALYTICS_ENGINEERS_GROUP
+ ENGINEERS_GROUP
end
- warn format(PRODUCT_ANALYTICS_CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(changed_files), product_analytics_engineers_group: mention)
+ warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(changed_files), engineers_group: mention)
warn format(UPDATE_METRICS_DEFINITIONS_MESSAGE) unless helper.changed_files(/usage_ping\.md/).any?
- product_analytics_labels = ['product analytics']
- product_analytics_labels << 'product analytics::review pending' unless helper.mr_has_labels?('product analytics::reviewed')
+ labels = ['product intelligence']
+ labels << 'product intelligence::review pending' unless helper.mr_has_labels?('product intelligence::approved')
- markdown(helper.prepare_labels_for_mr(product_analytics_labels))
+ markdown(helper.prepare_labels_for_mr(labels))
end
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index f49f6ad251c..424114a3d33 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -15,7 +15,8 @@ CATEGORY_TABLE_HEADER = <<MARKDOWN
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each
review slot, based on their timezone. Feel free to
[override these selections](https://about.gitlab.com/handbook/engineering/projects/#gitlab)
-if you think someone else would be better-suited, or the chosen person is unavailable.
+if you think someone else would be better-suited
+or use the [GitLab Review Workload Dashboard](https://gitlab-org.gitlab.io/gitlab-roulette/) to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the
[Engineering workflow](https://about.gitlab.com/handbook/engineering/workflow/#basics)
@@ -38,8 +39,8 @@ Please consider creating a merge request to
for them.
MARKDOWN
-OPTIONAL_REVIEW_TEMPLATE = "%{role} review is optional for %{category}".freeze
-NOT_AVAILABLE_TEMPLATE = 'No %{role} available'.freeze
+OPTIONAL_REVIEW_TEMPLATE = '%{role} review is optional for %{category}'
+NOT_AVAILABLE_TEMPLATE = 'No %{role} available'
def note_for_spins_role(spins, role)
spins.each do |spin|
diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile
index 72e0c8e92f4..26b52f64f2e 100644
--- a/danger/specs/Dangerfile
+++ b/danger/specs/Dangerfile
@@ -7,12 +7,12 @@ NO_SPECS_LABELS = [
'documentation',
'QA'
].freeze
-NO_NEW_SPEC_MESSAGE = <<~MSG
+NO_NEW_SPEC_MESSAGE = <<~MSG.freeze
You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the %<labels>s labels.
MSG
-EE_CHANGE_WITH_FOSS_SPEC_CHANGE_MESSAGE = <<~MSG
+EE_CHANGE_WITH_FOSS_SPEC_CHANGE_MESSAGE = <<~MSG.freeze
You've made some EE-specific changes, but only made changes to FOSS tests.
This could be a sign that you're testing an EE-specific behavior in a FOSS test.
@@ -24,6 +24,14 @@ Please make sure the spec files pass in AS-IF-FOSS mode either:
MSG
+CONTROLLER_SPEC_DEPRECATION_MESSAGE = <<~MSG.freeze
+Do not add new controller specs. We are moving from controller specs to
+request specs (and/or feature specs). Please add request specs under
+`/spec/requests` and/or `/ee/spec/requests` instead.
+
+See https://gitlab.com/groups/gitlab-org/-/epics/5076 for information.
+MSG
+
has_app_changes = helper.all_changed_files.grep(%r{\A(app|lib|db/(geo/)?(post_)?migrate)/}).any?
has_ee_app_changes = helper.all_changed_files.grep(%r{\Aee/(app|lib|db/(geo/)?(post_)?migrate)/}).any?
spec_changes = helper.all_changed_files.grep(%r{\Aspec/})
@@ -39,3 +47,8 @@ end
if has_ee_app_changes && has_spec_changes && !(has_app_changes || has_ee_spec_changes)
warn format(EE_CHANGE_WITH_FOSS_SPEC_CHANGE_MESSAGE, spec_files: spec_changes.join(" "), mr_title: gitlab.mr_json['title']), sticky: false
end
+
+# Forbidding a new file addition under `/spec/controllers` or `/ee/spec/controllers`
+if git.added_files.grep(%r{^(ee/)?spec/controllers/}).any?
+ warn CONTROLLER_SPEC_DEPRECATION_MESSAGE
+end