diff options
Diffstat (limited to 'danger')
-rw-r--r-- | danger/changelog/Dangerfile | 10 | ||||
-rw-r--r-- | danger/documentation/Dangerfile | 38 | ||||
-rw-r--r-- | danger/metadata/Dangerfile | 6 | ||||
-rw-r--r-- | danger/plugins/sidekiq_queues.rb | 10 | ||||
-rw-r--r-- | danger/roulette/Dangerfile | 35 | ||||
-rw-r--r-- | danger/sidekiq_queues/Dangerfile | 27 | ||||
-rw-r--r-- | danger/specs/Dangerfile | 8 |
7 files changed, 105 insertions, 29 deletions
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index aa7a81555a3..f9e65bbf4c7 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -35,7 +35,11 @@ def check_changelog_yaml(path) fail "`title` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil? fail "`type` should be set, in #{gitlab.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil? - if yaml["merge_request"].nil? && !helper.security_mr? + return if helper.security_mr? + + cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch? + + if yaml["merge_request"].nil? mr_line = raw_file.lines.find_index("merge_request:\n") if mr_line @@ -43,14 +47,14 @@ def check_changelog_yaml(path) else message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}" end - elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !helper.security_mr? + elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}" end rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias # YAML could not be parsed, fail the build. fail "#{gitlab.html_link(path)} isn't valid YAML! #{SEE_DOC}" rescue StandardError => e - warn "There was a problem trying to check the Changelog. Exception: #{e.name} - #{e.message}" + warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}" end def check_changelog_path(path) diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 1e27f9779e8..1dd6d484968 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -1,27 +1,33 @@ # frozen_string_literal: true +def gitlab_danger + @gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper) +end + docs_paths_to_review = helper.changes_by_category[:docs] -unless docs_paths_to_review.empty? - message 'This merge request adds or changes files that require a review ' \ - 'from the Technical Writing team.' +return if docs_paths_to_review.empty? + +message 'This merge request adds or changes files that require a review ' \ + 'from the Technical Writing team.' + +return unless gitlab_danger.ci? - 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 + - [Technical Writers assignments](https://about.gitlab.com/handbook/engineering/technical-writing/#designated-technical-writers) 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 - end +unless gitlab.mr_labels.include?('documentation') + gitlab.api.update_merge_request(gitlab.mr_json['project_id'], + gitlab.mr_json['iid'], + labels: (gitlab.mr_labels + ['documentation']).join(',')) end diff --git a/danger/metadata/Dangerfile b/danger/metadata/Dangerfile index b3313674951..1d7f863c201 100644 --- a/danger/metadata/Dangerfile +++ b/danger/metadata/Dangerfile @@ -5,7 +5,11 @@ THROUGHPUT_LABELS = [ 'security', 'bug', 'feature', - 'backstage', + 'feature::addition', + 'feature::maintenance', + 'tooling', + 'tooling::pipelines', + 'tooling::workflow', 'documentation' ].freeze diff --git a/danger/plugins/sidekiq_queues.rb b/danger/plugins/sidekiq_queues.rb new file mode 100644 index 00000000000..1edeb6da3d5 --- /dev/null +++ b/danger/plugins/sidekiq_queues.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require_relative '../../lib/gitlab/danger/sidekiq_queues' + +module Danger + class SidekiqQueues < Plugin + # Put the helper code somewhere it can be tested + include Gitlab::Danger::SidekiqQueues + end +end diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 54559af4066..6c192c3a311 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -12,17 +12,19 @@ MARKDOWN CATEGORY_TABLE_HEADER = <<MARKDOWN -To spread load more evenly across eligible reviewers, Danger has randomly picked -a candidate for each review slot. Feel free to +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. 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) and [code review guidelines](https://docs.gitlab.com/ee/development/code_review.html). +Please consider assigning a reviewer or maintainer who is a +[domain expert](https://about.gitlab.com/handbook/engineering/projects/#gitlab) in the area of the merge request. Once you've decided who will review this merge request, mention them as you -normally would! Danger does not (yet?) automatically notify them for you. +normally would! Danger does not automatically notify them for you. | Category | Reviewer | Maintainer | | -------- | -------- | ---------- | @@ -38,13 +40,18 @@ MARKDOWN OPTIONAL_REVIEW_TEMPLATE = "%{role} review is optional for %{category}".freeze NOT_AVAILABLE_TEMPLATE = 'No %{role} available'.freeze +TIMEZONE_EXPERIMENT = true + +def mr_author + roulette.team.find { |person| person.username == gitlab.mr_author } +end def note_for_category_role(spin, role) if spin.optional_role == role return OPTIONAL_REVIEW_TEMPLATE % { role: role.capitalize, category: helper.label_for_category(spin.category) } end - spin.public_send(role)&.markdown_name || NOT_AVAILABLE_TEMPLATE % { role: role } # rubocop:disable GitlabSecurity/PublicSend + spin.public_send(role)&.markdown_name(timezone_experiment: TIMEZONE_EXPERIMENT, author: mr_author) || NOT_AVAILABLE_TEMPLATE % { role: role } # rubocop:disable GitlabSecurity/PublicSend end def markdown_row_for_spin(spin) @@ -58,6 +65,8 @@ changes = helper.changes_by_category # Ignore any files that are known but uncategorized. Prompt for any unknown files changes.delete(:none) +# To reinstate roulette for documentation, remove this line. +changes.delete(:docs) categories = changes.keys - [:unknown] # Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries) @@ -67,12 +76,22 @@ if changes.any? project = helper.project_name branch_name = gitlab.mr_json['source_branch'] - roulette_spins = roulette.spin(project, categories, branch_name) - rows = roulette_spins.map { |spin| markdown_row_for_spin(spin) } + markdown(MESSAGE) - unknown = changes.fetch(:unknown, []) + roulette_spins = roulette.spin(project, categories, branch_name, timezone_experiment: TIMEZONE_EXPERIMENT) + rows = roulette_spins.map do |spin| + # MR includes QA changes, but also other changes, and author isn't an SET + if spin.category == :qa && categories.size > 1 && mr_author && !mr_author.reviewer?(project, spin.category, []) + spin.optional_role = :maintainer + end + + spin.optional_role = :maintainer if spin.category == :test + + markdown_row_for_spin(spin) + end - markdown(MESSAGE) markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty? + + unknown = changes.fetch(:unknown, []) markdown(UNKNOWN_FILES_MESSAGE + helper.markdown_list(unknown)) unless unknown.empty? end diff --git a/danger/sidekiq_queues/Dangerfile b/danger/sidekiq_queues/Dangerfile new file mode 100644 index 00000000000..b40be7a486e --- /dev/null +++ b/danger/sidekiq_queues/Dangerfile @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +SCALABILITY_REVIEW_MESSAGE = <<~MSG +## Sidekiq queue changes + +This merge request contains changes to Sidekiq queues. Please follow the [documentation on changing a queue's urgency](https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#changing-a-queues-urgency). +MSG + +ADDED_QUEUES_MESSAGE = <<~MSG +These queues were added: +MSG + +CHANGED_QUEUES_MESSAGE = <<~MSG +These queues had their attributes changed: +MSG + +if sidekiq_queues.added_queue_names.any? || sidekiq_queues.changed_queue_names.any? + markdown(SCALABILITY_REVIEW_MESSAGE) + + if sidekiq_queues.added_queue_names.any? + markdown(ADDED_QUEUES_MESSAGE + helper.markdown_list(sidekiq_queues.added_queue_names)) + end + + if sidekiq_queues.changed_queue_names.any? + markdown(CHANGED_QUEUES_MESSAGE + helper.markdown_list(sidekiq_queues.changed_queue_names)) + end +end diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile index 784ce75fef8..72e0c8e92f4 100644 --- a/danger/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -1,6 +1,12 @@ # frozen_string_literal: true -NO_SPECS_LABELS = %w[backstage documentation QA].freeze +NO_SPECS_LABELS = [ + 'tooling', + 'tooling::pipelines', + 'tooling::workflow', + 'documentation', + 'QA' +].freeze NO_NEW_SPEC_MESSAGE = <<~MSG You've made some app changes, but didn't add any tests. That's OK as long as you're refactoring existing code, |