From 34259e90cb840e4f00d82e3053ecc7b1baa45aed Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Thu, 18 Jun 2020 08:30:42 -0700 Subject: [PATCH] build: determine pullapprove fallback with better active group counting (#37636) Previously, the fallback group simply determined its active state based on the of active groups during processing. Instead, we should consider if there are any active groups after excluding the ones we don't want to be considered, i.e. the minimum required review group and the global approval groups. This allows for an explicit test of the active groups that exist rather than a prone to get out of date number of expected active groups. With this change, we additionally need to check to ensure that global approvals have not caused other groups to no longer be active, causing a false case of no active groups, when they have simply been superceded by a global approval. PR Close #37636 --- .pullapprove.yml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.pullapprove.yml b/.pullapprove.yml index eaa5197831..7cfc56b604 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -1222,13 +1222,17 @@ groups: # `global-approvers` can still approve PRs that match this `fallback` rule, # but that should be an exception and not an expectation. conditions: - - *can-be-global-approved - # The following groups have no conditions and will be `active` on all PRs + # The following groups have no file based conditions and will be initially `active` on all PRs # - `global-approvers` # - `global-docs-approvers` + # - `required-minimum-review` # - # Since this means the minimum number of active groups a PR can have is 2, this - # `fallback` group should be matched anytime the number of active groups is at or - # below this minimum. This work as a protection to ensure that pullapprove does - # not incidently mark a PR as passing without meeting the review criteria. - - len(groups.active) <= 2 + # By checking the number of active groups when these are excluded, we can determine + # if any other groups are matched. + - len(groups.active.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0 + # When any of the `global-*` groups is approved, they cause other groups to deactivate. + # In those cases, the condition above would evaluate to `true` while in reality, only a global + # approval has been provided. To ensure we don't activate the fallback group in such cases, + # ensure that no explicit global approval has been provided. + - *can-be-global-approved + - *can-be-global-docs-approved