From 4308f08e0c033d5ef6068edb0f84c44ff4194695 Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Tue, 28 Apr 2020 09:25:45 -0400 Subject: [PATCH 1/4] ci(danger): UX reviewers for component changes When an MR changes one or more components (and has the corresponding component labels), Danger will suggest a UX reviewer. --- danger/roulette/Dangerfile | 37 +++++++++++++++++++--- lib/gitlab/danger/helper.rb | 59 +++++++++++++++++++++++++++++++++++ lib/gitlab/danger/roulette.rb | 4 +++ 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index ae701dc2f9..c9eb645eea 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -43,9 +43,7 @@ rescue StandardError => e [] end -def spin_for_category(team, project, category, branch_name) - random = roulette.new_random(branch_name) - +def spin_for_category(team, project, category, random) reviewers, traintainers, maintainers = %i[reviewer? traintainer? maintainer?].map do |kind| team.select do |member| @@ -66,28 +64,57 @@ end changes = helper.changes_by_category categories = changes.keys +component_labels = helper.component_labels if changes.any? branch_name = gitlab.mr_json['source_branch'] + random = roulette.new_random(branch_name) team = team_for_project(GITLAB_PROJECT_ID) rows = categories.map do |category| - spin_for_category(team, GITLAB_PROJECT_ID, category, branch_name) + spin_for_category(team, GITLAB_PROJECT_ID, category, random) end # If these are docs-only changes, we need to pick a frontend maintainer for the final review if categories == %i[docs] - frontend_row = spin_for_category(team, GITLAB_PROJECT_ID, :frontend, branch_name) + frontend_row = spin_for_category(team, GITLAB_PROJECT_ID, :frontend, random) # We remove the frontend reviewer as the initial review will be performed by a technical writer frontend_row[:reviewer] = nil rows << frontend_row end + # Add component-specific UX reviewers + component_labels.each do |label| + ux_teammates = helper.ux_reviewers_for_label(label).map { |username| roulette.teammate_with_username(username) } + ux_reviewer = roulette.spin_for_person(ux_teammates, random: random) + component_ux_row = { + "label": "~\"component:#{label}\"", + "reviewer": ux_reviewer&.markdown_name, + "maintainer": nil + } + rows << component_ux_row + end + roulette_rows = rows.map do |row| "| #{row[:label]} | #{row[:reviewer] || NO_REVIEWER} | #{row[:maintainer] || NO_MAINTAINER} |" end markdown(MESSAGE) markdown(CATEGORY_TABLE_HEADER + roulette_rows.join("\n")) + unless component_labels.empty? + markdown( + <<~MARKDOWN + If your Merge Request changes one or more components, please have it reviewed by a Product Designer. + One should have been suggested above. Otherwise, or if they are not available, feel free to + assign to a UX Foundations designer: + + #{helper.foundations_ux_reviewers.map do |username| + roulette.teammate_with_username(username) + end.map do |teammate| + "* #{teammate&.markdown_name}" + end.join("\n")} + MARKDOWN + ) + end end diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 126a304485..e08e16d4c4 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -80,12 +80,71 @@ module Gitlab CATEGORY_LABELS.fetch(category, "~#{category}") end + def component_labels + gitlab_helper&.mr_labels.select { |label| /component:/ =~ label } + end + + def ux_reviewers_for_label(label) + component = label.sub! 'component:', '' + COMPONENT_UX_MAP[component.to_sym] || [] + end + + def foundations_ux_reviewers + %w[ + tauriedavis + jareko + jeldergl + ] + end + CATEGORY_LABELS = { docs: '~documentation' }.freeze CATEGORIES = { %r{\Adoc/} => :docs }.freeze + COMPONENT_UX_MAP = { + accordion: %w[kcomoli rayana], + alert: ['andyvolpe'], + avatar: ['tauriedavis'], + badge: ['pedroms'], + banner: ['andyvolpe'], + breadcrumb: ['ameliabauerly'], + 'broadcast-message': ['kmann'], + button: ['dimitrieh'], + card: ['beckalippert'], + chart: ['ameliabauerly'], + checkbox: ['pedroms'], + 'data-visualization': ['ameliabauerly'], + 'date-picker': ['tauriedavis'], + drawer: ['andyvolpe'], + dropdown: %w[hollyreynolds tauriedavis], + 'file-uploader': ['jareko'], + filter: ['matejlatin'], + form: ['tauriedavis'], + icon: ['jeldergl'], + 'infinite-scroll': ['beckalippert'], + label: ['annabeldunstone'], + link: ['jeldergl'], + list: ['tauriedavis'], + modal: ['dimitrieh'], + pagination: ['andyvolpe'], + popover: ['tauriedavis'], + 'progress-bar': ['jareko'], + radio: ['pedroms'], + search: ['matejlatin'], + 'segmented-control': ['andyvolpe'], + 'skeleton-loader': ['dimitrieh'], + sorting: ['ameliabauerly'], + spinner: ['dimitrieh'], + tab: ['dimitrieh'], + table: ['npost'], + toast: ['tauriedavis'], + toggle: ['pedroms'], + token: ['annabeldunstone'], + tooltip: ['rayana'], + tree: %w[kcomoli rayana] + }.freeze end end end diff --git a/lib/gitlab/danger/roulette.rb b/lib/gitlab/danger/roulette.rb index 45c6c372ab..d5d3577528 100644 --- a/lib/gitlab/danger/roulette.rb +++ b/lib/gitlab/danger/roulette.rb @@ -21,6 +21,10 @@ module Gitlab end end + def teammate_with_username(username) + team.find { |teammate| teammate.username == username } + end + # Like +team+, but only returns teammates in the current project, based on # project_name. # -- GitLab From 9ae849d394aac882dfa950bf21fc9515b730ae09 Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Fri, 8 May 2020 14:34:48 -0400 Subject: [PATCH 2/4] Use %w for all arrays --- lib/gitlab/danger/helper.rb | 74 ++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index e08e16d4c4..71dd4996e5 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -105,44 +105,44 @@ module Gitlab }.freeze COMPONENT_UX_MAP = { accordion: %w[kcomoli rayana], - alert: ['andyvolpe'], - avatar: ['tauriedavis'], - badge: ['pedroms'], - banner: ['andyvolpe'], - breadcrumb: ['ameliabauerly'], - 'broadcast-message': ['kmann'], - button: ['dimitrieh'], - card: ['beckalippert'], - chart: ['ameliabauerly'], - checkbox: ['pedroms'], - 'data-visualization': ['ameliabauerly'], - 'date-picker': ['tauriedavis'], - drawer: ['andyvolpe'], + alert: %w[andyvolpe], + avatar: %w[tauriedavis], + badge: %w[pedroms], + banner: %w[andyvolpe], + breadcrumb: %w[ameliabauerly], + 'broadcast-message': %w[kmann], + button: %w[dimitrieh], + card: %w[beckalippert], + chart: %w[ameliabauerly], + checkbox: %w[pedroms], + 'data-visualization': %w[ameliabauerly], + 'date-picker': %w[tauriedavis], + drawer: %w[andyvolpe], dropdown: %w[hollyreynolds tauriedavis], - 'file-uploader': ['jareko'], - filter: ['matejlatin'], - form: ['tauriedavis'], - icon: ['jeldergl'], - 'infinite-scroll': ['beckalippert'], - label: ['annabeldunstone'], - link: ['jeldergl'], - list: ['tauriedavis'], - modal: ['dimitrieh'], - pagination: ['andyvolpe'], - popover: ['tauriedavis'], - 'progress-bar': ['jareko'], - radio: ['pedroms'], - search: ['matejlatin'], - 'segmented-control': ['andyvolpe'], - 'skeleton-loader': ['dimitrieh'], - sorting: ['ameliabauerly'], - spinner: ['dimitrieh'], - tab: ['dimitrieh'], - table: ['npost'], - toast: ['tauriedavis'], - toggle: ['pedroms'], - token: ['annabeldunstone'], - tooltip: ['rayana'], + 'file-uploader': %w[jareko], + filter: %w[matejlatin], + form: %w[tauriedavis], + icon: %w[jeldergl], + 'infinite-scroll': %w[beckalippert], + label: %w[annabeldunstone], + link: %w[jeldergl], + list: %w[tauriedavis], + modal: %w[dimitrieh], + pagination: %w[andyvolpe], + popover: %w[tauriedavis], + 'progress-bar': %w[jareko], + radio: %w[pedroms], + search: %w[matejlatin], + 'segmented-control': %w[andyvolpe], + 'skeleton-loader': %w[dimitrieh], + sorting: %w[ameliabauerly], + spinner: %w[dimitrieh], + tab: %w[dimitrieh], + table: %w[npost], + toast: %w[tauriedavis], + toggle: %w[pedroms], + token: %w[annabeldunstone], + tooltip: %w[rayana], tree: %w[kcomoli rayana] }.freeze end -- GitLab From 25d11045d39bc0325cc909c8e88f426d91380166 Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Fri, 8 May 2020 14:37:31 -0400 Subject: [PATCH 3/4] Match labels with start_with? --- lib/gitlab/danger/helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 71dd4996e5..087afe1a58 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -81,7 +81,7 @@ module Gitlab end def component_labels - gitlab_helper&.mr_labels.select { |label| /component:/ =~ label } + gitlab_helper&.mr_labels.select { |label| label.start_with?('component:') } end def ux_reviewers_for_label(label) -- GitLab From 481d04af0009648fbef912dbb070190531859aa4 Mon Sep 17 00:00:00 2001 From: Paul Gascou-Vaillancourt Date: Fri, 8 May 2020 14:40:43 -0400 Subject: [PATCH 4/4] Don't modify the string in place --- danger/roulette/Dangerfile | 2 +- lib/gitlab/danger/helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index c9eb645eea..ae49329913 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -89,7 +89,7 @@ if changes.any? ux_teammates = helper.ux_reviewers_for_label(label).map { |username| roulette.teammate_with_username(username) } ux_reviewer = roulette.spin_for_person(ux_teammates, random: random) component_ux_row = { - "label": "~\"component:#{label}\"", + "label": "~\"#{label}\"", "reviewer": ux_reviewer&.markdown_name, "maintainer": nil } diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 087afe1a58..001277e85e 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -85,7 +85,7 @@ module Gitlab end def ux_reviewers_for_label(label) - component = label.sub! 'component:', '' + component = label.sub('component:', '') COMPONENT_UX_MAP[component.to_sym] || [] end -- GitLab