Pullapprove is marking PRs approved before they have sufficient reviews

[bzbarsky-apple]

We have not changed our .pullapprove.yml https://github.com/project-chip/connectedhomeip/blob/be248543eb11d956ba897285b2f558751b25e0a5/.pullapprove.yml at in a few months, and it has this bit:

    required-reviewers:
        requirements:
            - len(groups.approved.include('shared-reviewers-*')) >= 2

but PRs are being marked as approved with only one review. For example project-chip/connectedhomeip#25624 only has review from 1 person and the job listing says:

pullapprove — 1 group approved

but the pullapprove job has a green check, etc.

It looks like some people are being counted as being in shared-reviewers-signify even though it's defined like so:

    shared-reviewers-signify:
        type: optional
        conditions:
            - files.include('*')
        reviewers:
            teams: [reviewers-signify]
        reviews:
            request: 10

and the reviewers-signify team has 0 members. And then a review from one of them counts as "2 groups" even though pullapprove is still saying "1 group approved"...

[davegaeddert]

Hey @bzbarsky-apple — I think what's happening is that, because your "reviewers-signify" team is empty, it's putting all available users into the group (which is the same as what it does if you don't define any reviewers). I'll see if I can find a fix for that, but I assume you intend to put people on that team at some point? That should fix it too.

(the "1 group approved" message is referring to your required-reviewers group — all your other groups are optional and those aren't included in the status description)

[bzbarsky-apple]

@davegaeddert The team used to have users in it. It's managed by some set of people, and they removed users from their tea,.... But this should not break things for everyone else, ideally. ;)

For now as a workaround I disabled the relevant group entirely in our .pullapprove.yml, but in the meantime several PRs had merged with insufficient reviews....

[davegaeddert]

But this should not break things for everyone else, ideally. ;)

Agreed! I pushed a fix just a minute ago. You should be able to put that team back if you want. You can test it first to make sure that group comes up with no users: https://www.pullapprove.com/docs/v3/testing/