From f1300564a2b1531b8af6626cbb78d0450c141016 Mon Sep 17 00:00:00 2001 From: Jason Plum Date: Fri, 4 Aug 2023 20:05:18 +0000 Subject: [PATCH] Revert "Merge branch 'jcunha/add-reviewer-roulette-experiment' into 'master'" This reverts merge request !3252 --- .gitlab-ci.yml | 25 +++++++++++++++++-------- Dangerfile | 21 +++------------------ scripts/support/changelog/Dangerfile | 2 +- scripts/support/metadata/Dangerfile | 22 +++++++++++----------- scripts/support/reviewers/Dangerfile | 8 ++++---- 5 files changed, 36 insertions(+), 42 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a914329b23..4fe49ff8be 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -77,12 +77,6 @@ include: - template: Jobs/Secret-Detection.latest.gitlab-ci.yml - template: Jobs/SAST.latest.gitlab-ci.yml - template: Jobs/SAST-IaC.latest.gitlab-ci.yml - - project: 'gitlab-org/quality/pipeline-common' - file: - - '/ci/danger-review.yml' - rules: - - if: $CI_SERVER_HOST == "gitlab.com" - dependency_scanning: needs: [] before_script: [] @@ -465,8 +459,23 @@ debug_review_gke122: - if: '$PIPELINE_TYPE =~ /FEATURE_BRANCH_PIPELINE$/' danger-review: - before_script: - - bundle add gitlab-dangerfiles + image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger + stage: prepare + cache: {} + script: + - git version + - danger_id=$(echo -n ${DANGER_GITLAB_API_TOKEN} | md5sum | awk '{print $1}' | cut -c5-10) + - | + if [ ! -v DANGER_GITLAB_API_TOKEN ] && [ "${PIPELINE_TYPE}" = "FORK_MR_PIPELINE" ]; then + echo "Danger token not configured. As this is from a fork, failing danger gracefully." + exit 202 + fi + - danger --fail-on-errors=true --verbose --danger_id=${danger_id} + allow_failure: + exit_codes: 202 + rules: + - if: '$PIPELINE_TYPE =~ /MR_PIPELINE$/' + - if: '$PIPELINE_TYPE == "DOCS_PIPELINE"' rubocop: image: ruby:3.0-alpine diff --git a/Dangerfile b/Dangerfile index 5026390864..e119761617 100644 --- a/Dangerfile +++ b/Dangerfile @@ -1,18 +1,3 @@ -require "gitlab-dangerfiles" - -# Documentation reference: https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles -Gitlab::Dangerfiles.for_project(self, 'gitlab-chart') do |dangerfiles| - # Import all plugins from the gem - dangerfiles.import_plugins - - # Import a defined set of danger rules - dangerfiles.import_dangerfiles(only: %w[simple_roulette z_retry_link]) - - # These custom files have the potential to be either discarded or simplified, - # as the default rules available to us already provide much if not all of - # of our customization. Still, let's investigate this in a follow-up. - # For now, we can simply introduce the simple_roulette. - danger.import_dangerfile(path: 'scripts/support/changelog') - danger.import_dangerfile(path: 'scripts/support/metadata') - danger.import_dangerfile(path: 'scripts/support/reviewers') -end +danger.import_dangerfile(path: 'scripts/support/changelog') +danger.import_dangerfile(path: 'scripts/support/metadata') +danger.import_dangerfile(path: 'scripts/support/reviewers') diff --git a/scripts/support/changelog/Dangerfile b/scripts/support/changelog/Dangerfile index abf4b12ee2..d2d4175b40 100644 --- a/scripts/support/changelog/Dangerfile +++ b/scripts/support/changelog/Dangerfile @@ -78,7 +78,7 @@ labels is available in #{SEE_DOC_WORKTYPE}. MSG -changelog_needed = (helper.mr_labels & NO_CHANGELOG_LABELS).empty? +changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? if changelog_needed checked = 0 diff --git a/scripts/support/metadata/Dangerfile b/scripts/support/metadata/Dangerfile index e325a76cb6..40fe9ca27b 100644 --- a/scripts/support/metadata/Dangerfile +++ b/scripts/support/metadata/Dangerfile @@ -13,43 +13,43 @@ WORKTYPE_LABELS = [ 'documentation' ].freeze -if helper.mr_description.size < 5 +if gitlab.mr_body.size < 5 fail "Please provide a proper merge request description." end -if helper.mr_labels.empty? +if gitlab.mr_labels.empty? fail "Please add labels to this merge request." end -warn "You may want to add ~group::distribution label to this MR for gitlab-insights" unless helper.mr_labels.include?("group::distribution") +warn "You may want to add ~group::distribution label to this MR for gitlab-insights" unless gitlab.mr_labels.include?("group::distribution") -if helper.mr_assignees.empty? +unless gitlab.mr_json["assignee"] warn "This merge request does not have any assignee yet. Setting an assignee clarifies who needs to take action on the merge request at any given time." end -has_milestone = !helper.mr_milestone.nil? +has_milestone = !gitlab.mr_json["milestone"].nil? unless has_milestone warn "This merge request does not refer to an existing milestone.", sticky: false end -has_pick_into_stable_label = helper.mr_labels.find { |label| label.start_with?('Pick into') } +has_pick_into_stable_label = gitlab.mr_labels.find { |label| label.start_with?('Pick into') } -if helper.mr_target_branch != "master" && !has_pick_into_stable_label +if gitlab.branch_for_base != "master" && !has_pick_into_stable_label warn "Most of the time, all merge requests should target `master`. Otherwise, please set the relevant `Pick into X.Y` label." end -has_qa_verified_label = helper.mr_labels.find { |label| label.start_with?('QA:verified') } -is_docs_only_branch = helper.mr_source_branch =~ /(^docs[\/-].*|.*-docs$)/ +has_qa_verified_label = gitlab.mr_labels.find { |label| label.start_with?('QA:verified') } +is_docs_only_branch = gitlab.branch_for_head =~ /(^docs[\/-].*|.*-docs$)/ unless has_qa_verified_label || is_docs_only_branch warn "Please check the QA job and compare with builds on master. If no new failures are reported in QA job, add 'QA:verified' label." end -if (helper.mr_labels & WORKTYPE_LABELS).empty? +if (gitlab.mr_labels & WORKTYPE_LABELS).empty? warn "This merge request is missing any [Work Type Classification labels](https://about.gitlab.com/handbook/engineering/metrics/#work-type-classification)." end -if helper.ci? && !gitlab.mr_json["force_remove_source_branch"] +unless gitlab.mr_json["force_remove_source_branch"] warn "If the source branch can be safely removed after this merge request is accepted, please consider setting the '[delete source branch when merge request is accepted](https://docs.gitlab.com/ee/user/project/merge_requests/getting_started.html#deleting-the-source-branch)' option." end diff --git a/scripts/support/reviewers/Dangerfile b/scripts/support/reviewers/Dangerfile index 37ea097486..debd97807a 100644 --- a/scripts/support/reviewers/Dangerfile +++ b/scripts/support/reviewers/Dangerfile @@ -1,13 +1,13 @@ # frozen_string_literal: true MESSAGE = <` to request the first review to someone. It's recommended that you pick the one suggested by the reviewre roulette. But you can ignore it and assign it to someone else if you see fit. +If from a community member, ask that the ~"Community contribution" label be added as well. -Merge requests are handled according to the workflow documented in our [handbook](https://about.gitlab.com/handbook/engineering/development/enablement/systems/distribution/merge_requests.html#experimental-workflow) and should receive a response within the limit documented in our [First-response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#first-response-slo). +Merge requests are handled according to the workflow documented in our [handbook](https://about.gitlab.com/handbook/engineering/development/enablement/systems/distribution/merge_requests.html) and should receive a response within the limit documented in our [First-response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#first-response-slo). -If you don't receive a response from the reviewer within the SLO, please mention `@gitlab-org\/distribution`, or one of our [Project Maintainers](https://about.gitlab.com/handbook/engineering/projects/#gitlab-chart) +If you don't receive a response, please mention `@gitlab-org\/distribution`, or one of our [Project Maintainers](https://about.gitlab.com/handbook/engineering/projects/#gitlab-chart) REV_EOF # Print maintainers message -- GitLab