diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index d07d652e601f631207763f7ff1b4101918733589..417b5889bcf76e25d1df3c197a751340ce1d06ee 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -38,24 +38,32 @@ MARKDOWN NO_REVIEWER = 'No reviewer available'.freeze NO_MAINTAINER = 'No maintainer available'.freeze -def spin_for_category(team, project, category, branch_name) - random = roulette.new_random(branch_name) - labels = gitlab.mr_labels +Spin = Struct.new(:reviewer, :maintainer) + +def spin_role_for_category(team, role, project, category) + team.select do |member| + member.public_send("#{role}?", project, category, gitlab.mr_labels) # rubocop:disable GitlabSecurity/PublicSend + end +end +def spin_for_category(team, project, category, branch_name) reviewers, traintainers, maintainers = - %i[reviewer? traintainer? maintainer?].map do |kind| - team.select do |member| - member.public_send(kind, project, category, labels) # rubocop:disable GitlabSecurity/PublicSend - end + %i[reviewer traintainer maintainer].map do |role| + spin_role_for_category(team, role, project, category) end # TODO: take CODEOWNERS into account? # https://gitlab.com/gitlab-org/gitlab/issues/26723 # Make traintainers have triple the chance to be picked as a reviewer + random = roulette.new_random(branch_name) reviewer = roulette.spin_for_person(reviewers + traintainers + traintainers, random: random) maintainer = roulette.spin_for_person(maintainers, random: random) + Spin.new(reviewer, maintainer) +end + +def markdown_row_for_category(category, reviewer, maintainer) "| #{helper.label_for_category(category)} | #{reviewer&.markdown_name || NO_REVIEWER} | #{maintainer&.markdown_name || NO_MAINTAINER} |" end @@ -85,8 +93,29 @@ if changes.any? && !gitlab.mr_labels.include?('CSS cleanup') project = helper.project_name unknown = changes.fetch(:unknown, []) + spin_per_category = categories.each_with_object({}) do |category, memo| + memo[category] = spin_for_category(team, project, category, canonical_branch_name) + end + + rows = spin_per_category.map do |category, spin| + reviewer = spin.reviewer + maintainer = spin.maintainer + + case category + when :test + if reviewer.nil? + # Fetch an already picked backend reviewer, or pick one otherwise + reviewer = spin_per_category[:backend]&.reviewer || spin_for_category(team, project, :backend, canonical_branch_name).reviewer + end + when :engineering_productivity + if maintainer.nil? + # Fetch an already picked backend maintainer, or pick one otherwise + maintainer = spin_per_category[:backend]&.maintainer || spin_for_category(team, project, :backend, canonical_branch_name).maintainer + end + end - rows = categories.map { |category| spin_for_category(team, project, category, canonical_branch_name) } + markdown_row_for_category(category, reviewer, maintainer) + end markdown(MESSAGE) markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty? diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 6bb46b1730f2a9b4036cfdf92a72f9c8d456d426..aa2737262befd24ed79e231f27d9beb07c053cea 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -100,6 +100,7 @@ def label_for_category(category) test: "~test ~Quality for `spec/features/*`", engineering_productivity: '~"Engineering Productivity" for CI, Danger' }.freeze + # First-match win, so be sure to put more specific regex at the top... CATEGORIES = { %r{\Adoc/} => :none, # To reinstate roulette for documentation, set to `:docs`. %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :none, # To reinstate roulette for documentation, set to `:docs`. @@ -145,9 +146,8 @@ def label_for_category(category) %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend, %r{\A(ee/)?(bin|config|generator_templates|lib|rubocop)/} => :backend, %r{\A(ee/)?spec/features/} => :test, - %r{\A(ee/)?spec/(?!javascripts|frontend)[^/]+} => :backend, - %r{\A(ee/)?vendor/(?!assets)[^/]+} => :backend, - %r{\A(ee/)?vendor/(languages\.yml|licenses\.csv)\z} => :backend, + %r{\A(ee/)?spec/} => :backend, + %r{\A(ee/)?vendor/} => :backend, %r{\A(Gemfile|Gemfile.lock|Rakefile)\z} => :backend, %r{\A[A-Z_]+_VERSION\z} => :backend, %r{\A\.rubocop(_todo)?\.yml\z} => :backend,