diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-20 14:34:42 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-20 14:34:42 +0000 |
commit | 9f46488805e86b1bc341ea1620b866016c2ce5ed (patch) | |
tree | f9748c7e287041e37d6da49e0a29c9511dc34768 /danger | |
parent | dfc92d081ea0332d69c8aca2f0e745cb48ae5e6d (diff) | |
download | gitlab-ce-9f46488805e86b1bc341ea1620b866016c2ce5ed.tar.gz |
Add latest changes from gitlab-org/gitlab@13-0-stable-ee
Diffstat (limited to 'danger')
-rw-r--r-- | danger/changelog/Dangerfile | 8 | ||||
-rw-r--r-- | danger/gitlab_ui_wg/Dangerfile | 62 | ||||
-rw-r--r-- | danger/roulette/Dangerfile | 9 | ||||
-rw-r--r-- | danger/specs/Dangerfile | 6 | ||||
-rw-r--r-- | danger/telemetry/Dangerfile | 27 | ||||
-rw-r--r-- | danger/utility_css/Dangerfile | 51 |
6 files changed, 75 insertions, 88 deletions
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 210fe24c1e3..7b8a096639d 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -17,7 +17,7 @@ If you want to create a changelog entry for GitLab EE, run the following instead bin/changelog --ee -m %<mr_iid>s "%<mr_title>s" ``` -Note: Merge requests with %<labels>s do not trigger this check. +If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message. MSG def check_changelog_yaml(path) @@ -57,7 +57,7 @@ end if git.modified_files.include?("CHANGELOG.md") fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels) + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title) end changelog_found = changelog.found @@ -66,6 +66,6 @@ if changelog_found check_changelog_yaml(changelog_found) check_changelog_path(changelog_found) elsif changelog.needed? - message "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels) + message "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n" + + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title) end diff --git a/danger/gitlab_ui_wg/Dangerfile b/danger/gitlab_ui_wg/Dangerfile deleted file mode 100644 index 672b1deecb3..00000000000 --- a/danger/gitlab_ui_wg/Dangerfile +++ /dev/null @@ -1,62 +0,0 @@ -FRONTEND_MAINTAINERS = %w[filipa iamphill psimyn sarahghp mishunov].freeze -UX_MAINTAINERS = %w[tauriedavis rverissimo].freeze -NO_REVIEWER = 'No reviewer available'.freeze - -def mention_single_codebase_approvers - canonical_branch_name = - roulette.canonical_branch_name(gitlab.mr_json['source_branch']) - - random = roulette.new_random(canonical_branch_name) - - frontend_maintainers = helper.new_teammates(FRONTEND_MAINTAINERS) - ux_maintainers = helper.new_teammates(UX_MAINTAINERS) - - rows = [] - - if gitlab.mr_labels.include?('frontend') - frontend_maintainer = - roulette.spin_for_person(frontend_maintainers, random: random) - - rows << "| ~frontend | #{frontend_maintainer&.markdown_name || NO_REVIEWER}" - end - - if gitlab.mr_labels.include?('UX') - ux_maintainers = - roulette.spin_for_person(ux_maintainers, random: random) - - rows << "| ~UX | #{ux_maintainers&.markdown_name || NO_REVIEWER}" - end - - if rows.empty? - backup_maintainer = frontend_maintainers.sample - - rows << "| ~frontend / ~UX | #{backup_maintainer.markdown_name}" - end - - markdown(<<~MARKDOWN.strip) - ## GitLab UI Working Group changes - - This merge request contains changes related to the work of [cleaning up CSS and creating - reusable components](https://gitlab.com/groups/gitlab-org/-/epics/950). - These changes will need to be reviewed and approved by the following engineers: - - | Category | Reviewer - |----------|--------- - #{rows.join("\n")} - - To make sure this happens, please follow these steps: - - 1. Add all of the mentioned users to the list of merge request approvals. - 2. Assign the merge request to the first person in the above list. - - If you are a reviewer, please follow these steps: - - 1. Review the merge request. If it is good to go, approve it. - 2. Once approved, assign to the next person in the above list. If you are - the last person in the list, merge the merge request. - MARKDOWN -end - -if gitlab.mr_labels.include?('CSS cleanup') - mention_single_codebase_approvers -end diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 417b5889bcf..d73f6bf4f10 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -13,8 +13,9 @@ 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. +a candidate for each review slot. 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) @@ -76,9 +77,7 @@ categories = changes.keys - [:unknown] # Ensure to spin for database reviewer/maintainer when ~database is applied (e.g. to review SQL queries) categories << :database if gitlab.mr_labels.include?('database') && !categories.include?(:database) -# CSS Clean up MRs are reviewed using a slightly different process, so we -# disable the review roulette for such MRs. -if changes.any? && !gitlab.mr_labels.include?('CSS cleanup') +if changes.any? # Strip leading and trailing CE/EE markers canonical_branch_name = roulette.canonical_branch_name(gitlab.mr_json['source_branch']) diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile index 50887c44b3e..7803fad8472 100644 --- a/danger/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -7,14 +7,10 @@ That's OK as long as you're refactoring existing code, but please consider adding any of the %<labels>s labels. MSG -def presented_no_changelog_labels - NO_SPECS_LABELS.map { |label| "~#{label}" }.join(', ') -end - has_app_changes = !helper.all_changed_files.grep(%r{\A(ee/)?(app|lib|db/(geo/)?(post_)?migrate)/}).empty? has_spec_changes = !helper.all_changed_files.grep(%r{\A(ee/)?spec/}).empty? new_specs_needed = (gitlab.mr_labels & NO_SPECS_LABELS).empty? if has_app_changes && !has_spec_changes && new_specs_needed - warn format(NO_NEW_SPEC_MESSAGE, labels: presented_no_changelog_labels), sticky: false + warn format(NO_NEW_SPEC_MESSAGE, labels: helper.labels_list(NO_SPECS_LABELS)), sticky: false end diff --git a/danger/telemetry/Dangerfile b/danger/telemetry/Dangerfile index f308fb206bb..c18a15fcb03 100644 --- a/danger/telemetry/Dangerfile +++ b/danger/telemetry/Dangerfile @@ -9,24 +9,27 @@ USAGE_DATA_FILES_MESSAGE = <<~MSG For the following files, a review from the [Data team and Telemetry team](https://gitlab.com/groups/gitlab-org/growth/telemetry/engineers/-/group_members?with_inherited_permissions=exclude) is recommended: MSG -usage_data_changed_files = git.modified_files.grep(%r{usage_data}) +tracking_files = [ + 'lib/gitlab/tracking.rb', + 'spec/lib/gitlab/tracking_spec.rb', + 'app/helpers/tracking_helper.rb', + 'spec/helpers/tracking_helper_spec.rb', + 'app/assets/javascripts/tracking.js', + 'spec/frontend/tracking_spec.js' + ] -def has_label?(label) - gitlab.mr_labels.include?(label) -end +usage_data_changed_files = git.modified_files.grep(%r{usage_data}) +snowplow_events_changed_files = git.modified_files & tracking_files -def labels_for_merge_request(labels) - labels_list = labels.map { |label| %Q{~"#{label}"} }.join(' ') - "/label #{labels_list}" -end +changed_files = (usage_data_changed_files + snowplow_events_changed_files) -if usage_data_changed_files.any? +if changed_files.any? warn format(TELEMETRY_CHANGED_FILES_MESSAGE) - markdown(USAGE_DATA_FILES_MESSAGE + helper.markdown_list(usage_data_changed_files)) + markdown(USAGE_DATA_FILES_MESSAGE + helper.markdown_list(changed_files)) telemetry_labels = ['telemetry'] - telemetry_labels << 'telemetry::review pending' unless has_label?('telemetry::reviewed') + telemetry_labels << 'telemetry::review pending' unless helper.mr_has_labels?('telemetry::reviewed') - markdown(labels_for_merge_request(telemetry_labels)) + markdown(helper.prepare_labels_for_mr(telemetry_labels)) end diff --git a/danger/utility_css/Dangerfile b/danger/utility_css/Dangerfile new file mode 100644 index 00000000000..4f56c40379e --- /dev/null +++ b/danger/utility_css/Dangerfile @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +common = 'app/assets/stylesheets/framework/common.scss' +utilities = 'app/assets/stylesheets/utilities.scss' + +def get_css_files(files, common_filepath, utilities_filepath) + files.select do |file| + file.include?(common_filepath) || + file.include?(utilities_filepath) + end +end + +changed_util_files = get_css_files(helper.all_changed_files.to_a, common, utilities) + +unless changed_util_files.empty? + + markdown(<<~MARKDOWN) + ## Changes to utility SCSS files + MARKDOWN + + if changed_util_files.include?(common) + + markdown(<<~MARKDOWN) + ### Addition to `#{common}` + + You have added a new rule to `#{common}`. Are you sure you need this rule? + + If it is a component class shared across items, could it be added to the component as a utility class or to the component's stylesheet? If not, you can ignore this warning. + + If it is a new utility class, is there another class that shares the same values in either this file or in `#{utilities}`? If not, please add it to `#{utilities}`, following the [Gitlab UI naming style](https://unpkg.com/browse/@gitlab/ui/src/scss/utilities.scss). + + MARKDOWN + + end + + if changed_util_files.include?(utilities) + markdown(<<~MARKDOWN) + ### Addition to `#{utilities}` + + You have added a new rule to `#{utilities}`. Are you sure you need this rule? + + If it is a component class shared across items, could it be added to the component as a utility class or to the component's stylesheet? If not, consider adding it to `#{common}` + + If it is a new utility class, is there another class that shares the same values in either this file or in `#{utilities}`? If not, please be sure this addition follows the [Gitlab UI naming style](https://unpkg.com/browse/@gitlab/ui/src/scss/utilities.scss) so it may be removed when these rules are included. See [Include gitlab-ui utility-class library](https://gitlab.com/gitlab-org/gitlab/issues/36857) for more about this project. + + MARKDOWN + end + + warn "This merge request adds a new rule to #{common} or #{utilities}." + +end |