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
This commit is contained in:
Joey Perrott 2020-06-18 08:30:42 -07:00 committed by Misko Hevery
parent d0c69d207c
commit 34259e90cb
1 changed files with 11 additions and 7 deletions

View File

@ -1222,13 +1222,17 @@ groups:
# `global-approvers` can still approve PRs that match this `fallback` rule, # `global-approvers` can still approve PRs that match this `fallback` rule,
# but that should be an exception and not an expectation. # but that should be an exception and not an expectation.
conditions: conditions:
- *can-be-global-approved # The following groups have no file based conditions and will be initially `active` on all PRs
# The following groups have no conditions and will be `active` on all PRs
# - `global-approvers` # - `global-approvers`
# - `global-docs-approvers` # - `global-docs-approvers`
# - `required-minimum-review`
# #
# Since this means the minimum number of active groups a PR can have is 2, this # By checking the number of active groups when these are excluded, we can determine
# `fallback` group should be matched anytime the number of active groups is at or # if any other groups are matched.
# below this minimum. This work as a protection to ensure that pullapprove does - len(groups.active.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0
# not incidently mark a PR as passing without meeting the review criteria. # When any of the `global-*` groups is approved, they cause other groups to deactivate.
- len(groups.active) <= 2 # 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