Skip to content

Commit

Permalink
ci: no longer use deprecated pullapprove_conditions option (#42895)
Browse files Browse the repository at this point in the history
PullApprove deprecated the `pullapprove_conditions` config
option and introduced the `overrides` option. This commit
migrates to the new option, while also eliminating the
`fallback` group with a simple override (as per recommendation
from the pull approve docs).

PR Close #42895
  • Loading branch information
devversion authored and dylhunn committed Jul 21, 2021
1 parent 879bf22 commit 4307d6e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 69 deletions.
74 changes: 16 additions & 58 deletions .pullapprove.yml
Expand Up @@ -79,7 +79,10 @@ meta:
# one are those that appear above it.
no-groups-above-this-pending: &no-groups-above-this-pending len(groups.active.pending.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0
no-groups-above-this-rejected: &no-groups-above-this-rejected len(groups.active.rejected.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0
no-groups-above-this-active: &no-groups-above-this-active len(groups.active.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0

# Yaml anchor that represents a PullApprove expression that evaluates to `true` whenever
# there are no actual review groups active. Groups which are always active are excluded.
no-review-groups-active: &no-review-groups-active len(groups.active.exclude("required-minimum-review").exclude("global-approvers").exclude("global-docs-approvers")) == 0

can-be-global-approved: &can-be-global-approved '"global-approvers" not in groups.approved'
can-be-global-docs-approved: &can-be-global-docs-approved '"global-docs-approvers" not in groups.approved'
Expand All @@ -97,22 +100,23 @@ meta:
# https://developer.github.com/v3/previews/#draft-pull-requests
github_api_version: 'shadow-cat-preview'

pullapprove_conditions:
# https://docs.pullapprove.com/config/overrides/
# Note that overrides are processed in order.
overrides:
# For PRs which are still being worked on, either still in draft mode or indicated through WIP in
# title or label, PullApprove stays in a pending state until its ready for review.
- condition: "'WIP' not in title"
unmet_status: pending
explanation: 'Waiting to send reviews as PR is WIP'
- condition: "'PR state: WIP' not in labels"
unmet_status: pending
- if: "draft or 'WIP' in title or 'PR state: WIP' in labels"
status: pending
explanation: 'Waiting to send reviews as PR is WIP'
- condition: 'not draft'
unmet_status: pending
explanation: 'Waiting to send reviews as PR is in draft'
# Disable PullApprove on specific PRs by adding the `PullApprove: disable` label
- condition: "'PullApprove: disable' not in labels"
unmet_status: success
- if: "'PullApprove: disable' in labels"
status: success
explanation: "PullApprove skipped because of 'PullApprove: disable' label"
# If no groups are active, report this pull request as failing. Most likely, the
# PR author would need to update the PullApprove config, or create new group.
- if: *no-review-groups-active
status: failure
explanation: 'At least one group must match this PR. Please update an existing review group, or create a new group.'

groups:
# =========================================================
Expand Down Expand Up @@ -1305,49 +1309,3 @@ groups:
- atscott
- jelbourn
- josephperrott

# ====================================================
# Catch all for if no groups match the code change
# ====================================================
fallback:
<<: *defaults
# A group is considered to be `active` for a PR if at least one of group's
# conditions matches the PR.
#
# The PullApprove CI check should fail if a PR has no `active` groups, as
# this indicates the PR is modifying a file that has no owner.
#
# This is enforced through the pullapprove verification check done
# as part of the CircleCI lint job. Failures in this lint job should be
# fixed as part of the PR. This can be done by updating the
# `.pullapprove.yml` file cover the unmatched path.
# The pullapprove verification script is part of the ng-dev tool and can be
# run locally with the command: `yarn -s ng-dev pullapprove verify`
#
# For cases in which the verification check fails to ensure coverage, this
# group will be active. The expectation is that this should be remedied
# before merging the PR as described above. In an emergency situation
# `global-approvers` can still approve PRs that match this `fallback` rule,
# but that should be an exception and not an expectation.
conditions:
- *no-groups-above-this-active
# 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
# PullApprove uses a combination of users defined in the pullapprove configuration and the
# number of users who have performed reviews on Github in the recent past if the configuration
# does not specify it. Because, as an open source project, anyone on Github can perform a
# review we need to ensure that all groups, including the fallback group, have at least one user
# or group defined as reviewers.
reviewers:
users:
- josephperrott
reviews:
request: 0
required: 1
# Reviewed-for is required on fallback as it should not ever actually be reviewed, requiring
# Reviewed-for helps insure an accidental approval doesn't occur on the fallback group.
reviewed_for: required
6 changes: 1 addition & 5 deletions dev-infra/ng-dev.js
Expand Up @@ -5180,10 +5180,6 @@ function transformExpressionToJs(expression) {
*/
// Regular expression that matches conditions for the global approval.
const GLOBAL_APPROVAL_CONDITION_REGEX = /^"global-(docs-)?approvers" not in groups.approved$/;
// Name of the PullApprove group that serves as fallback. This group should never capture
// any conditions as it would always match specified files. This is not desired as we want
// to figure out as part of this tool, whether there actually are unmatched files.
const FALLBACK_GROUP_NAME = 'fallback';
/** A PullApprove group to be able to test files against. */
class PullApproveGroup {
constructor(groupName, config, precedingGroups = []) {
Expand All @@ -5196,7 +5192,7 @@ class PullApproveGroup {
this.reviewers = (_a = config.reviewers) !== null && _a !== void 0 ? _a : { users: [], teams: [] };
}
_captureConditions(config) {
if (config.conditions && this.groupName !== FALLBACK_GROUP_NAME) {
if (config.conditions) {
return config.conditions.forEach(condition => {
const expression = condition.trim();
if (expression.match(GLOBAL_APPROVAL_CONDITION_REGEX)) {
Expand Down
7 changes: 1 addition & 6 deletions dev-infra/pullapprove/group.ts
Expand Up @@ -37,11 +37,6 @@ export interface PullApproveGroupResult {
// Regular expression that matches conditions for the global approval.
const GLOBAL_APPROVAL_CONDITION_REGEX = /^"global-(docs-)?approvers" not in groups.approved$/;

// Name of the PullApprove group that serves as fallback. This group should never capture
// any conditions as it would always match specified files. This is not desired as we want
// to figure out as part of this tool, whether there actually are unmatched files.
const FALLBACK_GROUP_NAME = 'fallback';

/** A PullApprove group to be able to test files against. */
export class PullApproveGroup {
/** List of conditions for the group. */
Expand All @@ -57,7 +52,7 @@ export class PullApproveGroup {
}

private _captureConditions(config: PullApproveGroupConfig) {
if (config.conditions && this.groupName !== FALLBACK_GROUP_NAME) {
if (config.conditions) {
return config.conditions.forEach(condition => {
const expression = condition.trim();

Expand Down

0 comments on commit 4307d6e

Please sign in to comment.