summaryrefslogtreecommitdiff
path: root/danger
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-02-05 17:16:18 +0000
committerNick Thomas <nick@gitlab.com>2019-02-13 16:41:28 +0000
commit77b2ecd2b1de3f66c3d9ebd5e58fedafb17ea606 (patch)
treeeb360eb0264b50370ff3ad48910ad437183bc3b6 /danger
parent235fe67bdbc7f177a6f4b213bb1780f043944246 (diff)
downloadgitlab-ce-77b2ecd2b1de3f66c3d9ebd5e58fedafb17ea606.tar.gz
Reviewer roulette via Danger56087-danger-roulette
Make danger pick reviewers and maintainers at random, for feontend, backend, database, etc, changes, whenever files belonging to those teams get changed.
Diffstat (limited to 'danger')
-rw-r--r--danger/documentation/Dangerfile2
-rw-r--r--danger/plugins/helper.rb59
-rw-r--r--danger/roulette/Dangerfile78
3 files changed, 88 insertions, 51 deletions
diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile
index 76e81f5f62f..dd0e3f6deb6 100644
--- a/danger/documentation/Dangerfile
+++ b/danger/documentation/Dangerfile
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-docs_paths_to_review = helper.changes_by_category[:documentation]
+docs_paths_to_review = helper.changes_by_category[:documentation]
unless docs_paths_to_review.empty?
message 'This merge request adds or changes files that require a ' \
diff --git a/danger/plugins/helper.rb b/danger/plugins/helper.rb
index 09ee50a2ef0..581c0720083 100644
--- a/danger/plugins/helper.rb
+++ b/danger/plugins/helper.rb
@@ -1,56 +1,15 @@
# frozen_string_literal: true
-module Danger
- # Common helper functions for our danger scripts
- # If we find ourselves repeating code in our danger files, we might as well put them in here.
- class Helper < Plugin
- # Returns a list of all files that have been added, modified or renamed.
- # `git.modified_files` might contain paths that already have been renamed,
- # so we need to remove them from the list.
- #
- # Considering these changes:
- #
- # - A new_file.rb
- # - D deleted_file.rb
- # - M modified_file.rb
- # - R renamed_file_before.rb -> renamed_file_after.rb
- #
- # it will return
- # ```
- # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ]
- # ```
- #
- # @return [Array<String>]
- def all_changed_files
- Set.new
- .merge(git.added_files.to_a)
- .merge(git.modified_files.to_a)
- .merge(git.renamed_files.map { |x| x[:after] })
- .subtract(git.renamed_files.map { |x| x[:before] })
- .to_a
- .sort
- end
-
- # @return [Boolean]
- def ee?
- ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md')
- end
-
- # @return [Hash<String,Array<String>>]
- def changes_by_category
- all_changed_files.inject(Hash.new { |h, k| h[k] = [] }) do |hsh, file|
- hsh[category_for_file(file)] << file
- end
- end
+require 'net/http'
+require 'yaml'
- def category_for_file(file)
- _, category = CATEGORIES.find { |regexp, _| regexp.match?(file) }
+require_relative '../../lib/gitlab/danger/helper'
- category || :unknown
- end
-
- CATEGORIES = {
- %r{\Adoc/} => :documentation
- }
+module Danger
+ # Common helper functions for our danger scripts. See Gitlab::Danger::Helper
+ # for more details
+ class Helper < Plugin
+ # Put the helper code somewhere it can be tested
+ include Gitlab::Danger::Helper
end
end
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
new file mode 100644
index 00000000000..5c3d7a4ca49
--- /dev/null
+++ b/danger/roulette/Dangerfile
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+MESSAGE = <<MARKDOWN
+## Reviewer roulette
+
+Changes that require review have been detected! A merge request is normally
+reviewed by both a reviewer and a maintainer in its primary category (e.g.
+~frontend or ~backend), and by a maintainer in all other categories.
+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 override this selection if you
+think someone else would be better-suited, or the chosen person is unavailable.
+
+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.
+
+| Category | Reviewer | Maintainer |
+| -------- | -------- | ---------- |
+MARKDOWN
+
+UNKNOWN_FILES_MESSAGE = <<MARKDOWN
+
+These files couldn't be categorised, so Danger was unable to suggest a reviewer.
+Please consider creating a merge request to
+[add support](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/danger/helper.rb)
+for them.
+MARKDOWN
+
+def spin(team, project, category)
+ reviewers = team.select { |member| member.reviewer?(project, category) }
+ maintainers = team.select { |member| member.maintainer?(project, category) }
+
+ # TODO: filter out people who are currently not in the office
+ # TODO: take CODEOWNERS into account?
+
+ reviewer = reviewers[rand(reviewers.size)]
+ maintainer = maintainers[rand(maintainers.size)]
+
+ "| #{helper.label_for_category(category)} | #{reviewer&.markdown_name} | #{maintainer&.markdown_name} |"
+end
+
+def build_list(items)
+ list = items.map { |filename| "* `#{filename}`" }.join("\n")
+
+ if items.size > 10
+ "\n<details>\n\n#{list}\n\n</details>"
+ else
+ list
+ end
+end
+
+changes = helper.changes_by_category
+categories = changes.keys - [:unknown]
+
+unless changes.empty?
+ team =
+ begin
+ helper.project_team
+ rescue => err
+ warn("Reviewer roulette failed to load team data: #{err.message}")
+ []
+ end
+
+ # Exclude the MR author from the team for selection purposes
+ team.delete_if { |teammate| teammate.username == gitlab.mr_author }
+
+ project = helper.project_name
+ unknown = changes.fetch(:unknown, [])
+
+ rows = categories.map { |category| spin(team, project, category) }
+
+ markdown(MESSAGE)
+ markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty?
+ markdown(UNKNOWN_FILES_MESSAGE + build_list(unknown)) unless unknown.empty?
+end