diff options
Diffstat (limited to 'danger')
| -rw-r--r-- | danger/changelog/Dangerfile | 2 | ||||
| -rw-r--r-- | danger/changes_size/Dangerfile | 2 | ||||
| -rw-r--r-- | danger/commit_messages/Dangerfile | 8 | ||||
| -rw-r--r-- | danger/feature_flag/Dangerfile | 78 | ||||
| -rw-r--r-- | danger/frozen_string/Dangerfile | 47 | ||||
| -rw-r--r-- | danger/metadata/Dangerfile | 6 | ||||
| -rw-r--r-- | danger/pipeline/Dangerfile | 7 | ||||
| -rw-r--r-- | danger/plugins/changelog.rb | 4 | ||||
| -rw-r--r-- | danger/plugins/feature_flag.rb | 10 | ||||
| -rw-r--r-- | danger/plugins/helper.rb | 6 | ||||
| -rw-r--r-- | danger/plugins/roulette.rb | 4 | ||||
| -rw-r--r-- | danger/plugins/sidekiq_queues.rb | 4 | ||||
| -rw-r--r-- | danger/product_intelligence/Dangerfile | 39 | ||||
| -rw-r--r-- | danger/roulette/Dangerfile | 6 |
14 files changed, 145 insertions, 78 deletions
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 06593a04093..c8474157fa5 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -35,7 +35,7 @@ def check_changelog_yaml(path) elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}" end -rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias +rescue Psych::Exception # YAML could not be parsed, fail the build. fail "#{gitlab.html_link(path)} isn't valid YAML! #{SEE_DOC}" rescue StandardError => e diff --git a/danger/changes_size/Dangerfile b/danger/changes_size/Dangerfile index 2c1d59427af..f37632ced33 100644 --- a/danger/changes_size/Dangerfile +++ b/danger/changes_size/Dangerfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # FIXME: git.info_for_file raises the following error # /usr/local/bundle/gems/git-1.4.0/lib/git/lib.rb:956:in `command': (Danger::DSLError) # [!] Invalid `Dangerfile` file: diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile index 816d7384a2d..96a0c08c184 100644 --- a/danger/commit_messages/Dangerfile +++ b/danger/commit_messages/Dangerfile @@ -1,7 +1,7 @@ # frozen_string_literal: true -require_relative File.expand_path('../../lib/gitlab/danger/commit_linter', __dir__) -require_relative File.expand_path('../../lib/gitlab/danger/merge_request_linter', __dir__) +require_relative File.expand_path('../../tooling/danger/commit_linter', __dir__) +require_relative File.expand_path('../../tooling/danger/merge_request_linter', __dir__) COMMIT_MESSAGE_GUIDELINES = "https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#commit-messages-guidelines" MORE_INFO = "For more information, take a look at our [Commit message guidelines](#{COMMIT_MESSAGE_GUIDELINES})." @@ -54,7 +54,7 @@ end # https://github.com/jonallured/danger-commit_lint because its output is not # very helpful, and it doesn't offer the means of ignoring merge commits. def lint_commit(commit) - linter = Gitlab::Danger::CommitLinter.new(commit) + linter = Tooling::Danger::CommitLinter.new(commit) # For now we'll ignore merge commits, as getting rid of those is a problem # separate from enforcing good commit messages. @@ -93,7 +93,7 @@ end def lint_mr_title(mr_title) commit = Struct.new(:message, :sha).new(mr_title) - Gitlab::Danger::MergeRequestLinter.new(commit).lint + Tooling::Danger::MergeRequestLinter.new(commit).lint end def count_non_fixup_commits(commit_linters) diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile new file mode 100644 index 00000000000..c90f60640f2 --- /dev/null +++ b/danger/feature_flag/Dangerfile @@ -0,0 +1,78 @@ +# frozen_string_literal: true +# rubocop:disable Style/SignalException + +SEE_DOC = "See the [feature flag documentation](https://docs.gitlab.com/ee/development/feature_flags/development.html#feature-flag-definition-and-validation)." + +SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT +```suggestion +group: "%<group>s" +``` + +#{SEE_DOC} +SUGGEST_COMMENT + +def check_feature_flag_yaml(feature_flag) + mr_group_label = helper.group_label(gitlab.mr_labels) + + if feature_flag.group.nil? + message_for_feature_flag_missing_group!(feature_flag: feature_flag, mr_group_label: mr_group_label) + else + message_for_feature_flag_with_group!(feature_flag: feature_flag, mr_group_label: mr_group_label) + end +rescue Psych::Exception + # YAML could not be parsed, fail the build. + fail "#{gitlab.html_link(feature_flag.path)} isn't valid YAML! #{SEE_DOC}" +rescue StandardError => e + warn "There was a problem trying to check the Feature Flag file. Exception: #{e.class.name} - #{e.message}" +end + +def message_for_feature_flag_missing_group!(feature_flag:, mr_group_label:) + if mr_group_label.nil? + warn "Consider setting `group` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC}" + else + mr_line = feature_flag.raw.lines.find_index("group:\n") + + if mr_line + markdown(format(SUGGEST_MR_COMMENT, group: mr_group_label), file: feature_flag.path, line: mr_line.succ) + else + warn %(Consider setting `group: "#{mr_group_label}"` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC}) + end + end +end + +def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:) + return if feature_flag.group_match_mr_label?(mr_group_label) + + if mr_group_label.nil? + gitlab.api.update_merge_request(gitlab.mr_json['project_id'], + gitlab.mr_json['iid'], + add_labels: feature_flag.group) + else + fail %(`group` is set to ~"#{feature_flag.group}" in #{gitlab.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!) + end +end + +def feature_flag_file_added_or_removed? + feature_flag.feature_flag_files(change_type: :added).any? || feature_flag.feature_flag_files(change_type: :deleted).any? +end + +feature_flag.feature_flag_files(change_type: :added).each do |feature_flag| + check_feature_flag_yaml(feature_flag) +end + +if feature_flag_file_added_or_removed? + new_mr_title = helper.mr_title.dup + new_mr_title << ' [RUN ALL RSPEC]' unless helper.run_all_rspec_mr? + new_mr_title << ' [RUN AS-IF-FOSS]' unless helper.run_as_if_foss_mr? + + if new_mr_title != helper.mr_title + gitlab.api.update_merge_request( + gitlab.mr_json['project_id'], + gitlab.mr_json['iid'], + title: new_mr_title + ) + gitlab.api.post("/projects/#{gitlab.mr_json['project_id']}/merge_requests/#{gitlab.mr_json['iid']}/pipelines") + else + message "You're adding or removing a feature flag, your MR title needs to include `[RUN ALL RSPEC] [RUN AS-IF-FOSS]` (we may have updated it automatically for you and started a new MR pipeline) to ensure everything is covered." + end +end diff --git a/danger/frozen_string/Dangerfile b/danger/frozen_string/Dangerfile deleted file mode 100644 index bc598b16463..00000000000 --- a/danger/frozen_string/Dangerfile +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -FILE_EXTENSION = ".rb" -FROZEN_STRING_MAGIC_COMMENT = "# frozen_string_literal: true" -SHEBANG_COMMENT = "#!" - -def get_files_with_no_magic_comment(files) - files.select do |path| - path.end_with?(FILE_EXTENSION) && - !file_has_frozen_string_magic_comment?(path) - end -end - -def file_has_frozen_string_magic_comment?(path) - File.open(path) do |file| - first_line = file.gets - - line_has_frozen_string_magic_comment?(first_line) || - (line_has_shebang?(first_line) && - line_has_frozen_string_magic_comment?(file.gets)) - end -end - -def line_has_frozen_string_magic_comment?(line) - line&.start_with?(FROZEN_STRING_MAGIC_COMMENT) -end - -def line_has_shebang?(line) - line&.start_with?(SHEBANG_COMMENT) -end - -files_to_fix = get_files_with_no_magic_comment(git.added_files) - -if files_to_fix.any? - warn 'This merge request adds files that do not enforce frozen string literal. ' \ - 'See https://gitlab.com/gitlab-org/gitlab-foss/issues/47424 for more information.' - - if GitlabDanger.new(helper.gitlab_helper).ci? - markdown(<<~MARKDOWN) - ## Enable Frozen String Literal - - The following files should have `#{FROZEN_STRING_MAGIC_COMMENT}` on the first line: - - * #{files_to_fix.map { |path| "`#{path}`" }.join("\n* ")} - MARKDOWN - end -end diff --git a/danger/metadata/Dangerfile b/danger/metadata/Dangerfile index b98d10d0654..d74c4f4a5af 100644 --- a/danger/metadata/Dangerfile +++ b/danger/metadata/Dangerfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # rubocop:disable Style/SignalException THROUGHPUT_LABELS = [ @@ -17,10 +19,6 @@ if gitlab.mr_body.size < 5 fail "Please provide a proper merge request description." end -if gitlab.mr_labels.empty? - fail "Please add labels to this merge request." -end - if (THROUGHPUT_LABELS & gitlab.mr_labels).empty? warn 'Please add a [throughput label](https://about.gitlab.com/handbook/engineering/management/throughput/#implementation) to this merge request.' end diff --git a/danger/pipeline/Dangerfile b/danger/pipeline/Dangerfile index 0342a39554b..c71fb86325d 100644 --- a/danger/pipeline/Dangerfile +++ b/danger/pipeline/Dangerfile @@ -7,9 +7,12 @@ This Merge Request contains changes to the pipeline configuration for the GitLab Please consider the effect of the changes in this Merge Request on the following: - Effects on different [pipeline types](https://docs.gitlab.com/ee/development/pipelines.html#pipelines-for-merge-requests) -- Effects on non-canonical projects (`gitlab-foss`, `security`, etc) +- Effects on non-canonical projects: + - `gitlab-foss` + - `security` + - `dev` + - personal forks - Effects on [pipeline performance](https://about.gitlab.com/handbook/engineering/quality/performance-indicators/#average-merge-request-pipeline-duration-for-gitlab) -- Effects on fork pipelines Please consider communicating these changes to the broader team following the [communication guideline for pipeline changes](https://about.gitlab.com/handbook/engineering/quality/engineering-productivity-team/#pipeline-changes) MESSAGE diff --git a/danger/plugins/changelog.rb b/danger/plugins/changelog.rb index 84f399e9e97..fd2dad5932a 100644 --- a/danger/plugins/changelog.rb +++ b/danger/plugins/changelog.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require_relative '../../lib/gitlab/danger/changelog' +require_relative '../../tooling/danger/changelog' module Danger class Changelog < Plugin # Put the helper code somewhere it can be tested - include Gitlab::Danger::Changelog + include Tooling::Danger::Changelog end end diff --git a/danger/plugins/feature_flag.rb b/danger/plugins/feature_flag.rb new file mode 100644 index 00000000000..8db8daba6d9 --- /dev/null +++ b/danger/plugins/feature_flag.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require_relative '../../tooling/danger/feature_flag' + +module Danger + class FeatureFlag < Plugin + # Put the helper code somewhere it can be tested + include Tooling::Danger::FeatureFlag + end +end diff --git a/danger/plugins/helper.rb b/danger/plugins/helper.rb index 2d7a933e801..8602868d817 100644 --- a/danger/plugins/helper.rb +++ b/danger/plugins/helper.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true -require_relative '../../lib/gitlab/danger/helper' +require_relative '../../tooling/danger/helper' module Danger - # Common helper functions for our danger scripts. See Gitlab::Danger::Helper + # Common helper functions for our danger scripts. See Tooling::Danger::Helper # for more details class Helper < Plugin # Put the helper code somewhere it can be tested - include Gitlab::Danger::Helper + include Tooling::Danger::Helper end end diff --git a/danger/plugins/roulette.rb b/danger/plugins/roulette.rb index 7c62cff0c92..2aa0132852e 100644 --- a/danger/plugins/roulette.rb +++ b/danger/plugins/roulette.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require_relative '../../lib/gitlab/danger/roulette' +require_relative '../../tooling/danger/roulette' module Danger class Roulette < Plugin # Put the helper code somewhere it can be tested - include Gitlab::Danger::Roulette + include Tooling::Danger::Roulette end end diff --git a/danger/plugins/sidekiq_queues.rb b/danger/plugins/sidekiq_queues.rb index 1edeb6da3d5..dd436e5cb2b 100644 --- a/danger/plugins/sidekiq_queues.rb +++ b/danger/plugins/sidekiq_queues.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require_relative '../../lib/gitlab/danger/sidekiq_queues' +require_relative '../../tooling/danger/sidekiq_queues' module Danger class SidekiqQueues < Plugin # Put the helper code somewhere it can be tested - include Gitlab::Danger::SidekiqQueues + include Tooling::Danger::SidekiqQueues end end diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile index ec432544977..5fd5b962993 100644 --- a/danger/product_intelligence/Dangerfile +++ b/danger/product_intelligence/Dangerfile @@ -1,8 +1,9 @@ # frozen_string_literal: true +# rubocop:disable Style/SignalException CHANGED_FILES_MESSAGE = <<~MSG For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended -Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/product_analytics/usage_ping.html) and reach out to %<engineers_group>s group for a review. +Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/usage_ping.html) and reach out to %<engineers_group>s group for a review. %<changed_files>s @@ -16,21 +17,41 @@ MSG ENGINEERS_GROUP = '@gitlab-org/growth/product-intelligence/engineers' +UPDATE_DICTIONARY_MESSAGE = <<~MSG + When updating metrics definitions, please update [Metrics Dictionary](https://docs.gitlab.com/ee/development/usage_ping/dictionary.html) + + ```shell + bundle exec rake gitlab:usage_data:generate_metrics_dictionary + ``` +MSG + +all_changed_files = helper.all_changed_files + 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' + 'spec/frontend/tracking_spec.js', + 'generator_templates/usage_metric_definition/metric_definition.yml', + 'lib/generators/rails/usage_metric_definition_generator.rb', + 'spec/lib/generators/usage_metric_definition_generator_spec.rb', + 'config/metrics/schema.json' ] -usage_data_changed_files = helper.changed_files(/usage_data/) -snowplow_events_changed_files = git.modified_files & tracking_files +tracking_changed_files = all_changed_files & tracking_files +usage_data_changed_files = all_changed_files.grep(%r{(usage_data)}) +metrics_changed_files = all_changed_files.grep(%r{((ee/)?config/metrics/.*\.yml)}) +dictionary_changed_file = all_changed_files.grep(%r{(doc/development/usage_ping/dictionary.md)}) -changed_files = (usage_data_changed_files + snowplow_events_changed_files) +snowplow_changed_files = all_changed_files.select do |file| + helper.changed_lines(file).grep(%r{Gitlab::Tracking\.event}).any? +end + +matching_changed_files = usage_data_changed_files + tracking_changed_files + metrics_changed_files + dictionary_changed_file + snowplow_changed_files -if changed_files.any? +if matching_changed_files.any? mention = if helper.draft_mr? "`#{ENGINEERS_GROUP}`" @@ -38,8 +59,10 @@ if changed_files.any? ENGINEERS_GROUP end - warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(changed_files), engineers_group: mention) - warn format(UPDATE_METRICS_DEFINITIONS_MESSAGE) unless helper.changed_files(/usage_ping\.md/).any? + warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(matching_changed_files), engineers_group: mention) + warn format(UPDATE_METRICS_DEFINITIONS_MESSAGE) if usage_data_changed_files.any? + + fail format(UPDATE_DICTIONARY_MESSAGE) if metrics_changed_files.any? && dictionary_changed_file.empty? labels = ['product intelligence'] labels << 'product intelligence::review pending' unless helper.mr_has_labels?('product intelligence::approved') diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 424114a3d33..3096ef471dd 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -24,8 +24,8 @@ and [code review guidelines](https://docs.gitlab.com/ee/development/code_review. Please consider assigning a reviewer or maintainer who is a [domain expert](https://about.gitlab.com/handbook/engineering/projects/#gitlab) in the area of the merge request. -Once you've decided who will review this merge request, mention them as you -normally would! Danger does not automatically notify them for you. +Once you've decided who will review this merge request, assign them as a reviewer! +Danger does not automatically notify them for you. | Category | Reviewer | Maintainer | | -------- | -------- | ---------- | @@ -35,7 +35,7 @@ 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/blob/master/lib/gitlab/danger/helper.rb) +[add support](https://gitlab.com/gitlab-org/gitlab/blob/master/tooling/danger/helper.rb) for them. MARKDOWN |