diff options
author | Nick Thomas <nick@gitlab.com> | 2019-02-05 17:16:18 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-02-13 16:41:28 +0000 |
commit | 77b2ecd2b1de3f66c3d9ebd5e58fedafb17ea606 (patch) | |
tree | eb360eb0264b50370ff3ad48910ad437183bc3b6 /danger | |
parent | 235fe67bdbc7f177a6f4b213bb1780f043944246 (diff) | |
download | gitlab-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/Dangerfile | 2 | ||||
-rw-r--r-- | danger/plugins/helper.rb | 59 | ||||
-rw-r--r-- | danger/roulette/Dangerfile | 78 |
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 |