diff options
Diffstat (limited to 'danger')
-rw-r--r-- | danger/frozen_string/Dangerfile | 29 | ||||
-rw-r--r-- | danger/product_intelligence/Dangerfile (renamed from danger/product_analytics/Dangerfile) | 20 | ||||
-rw-r--r-- | danger/roulette/Dangerfile | 7 | ||||
-rw-r--r-- | danger/specs/Dangerfile | 17 |
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 |