I don't think that this is right. or is it? we care if we have all the approvals. no? if someone objects and has the authority to do so, we should change pullapprove to not approve the PR.

the thing is that anyone can object to a change, including a non-contributor. this should not block merging.

Here is the use case.

1. I approve the PR, pull approve goes green 💚 .
2. You come along and object. You would expect for pull approve to go red 🔴 , but it does not. So now we are in mergeable state even touch there are outstanding reviews and that should not be it.

At this point status is green with a check mark but you have an objection. This PR makes it so that both the pull approve status as well as reviews are green. (Yes it is weird that status can be green and review red, but that is the way it is)

So we could be smarter and try to reconcile which reviews are from our team and which are not, but that would make the script complicated. The easiest is to just go to the UI and dismiss the review which you don't agree with.

how about we use http://docs.pullapprove.com/groups/reject_value/ instead?

required: 1
reject_value: -999

that way pullapprove is always the source of truth and it's up to the reviewer/pr author to make sure that the PR is green.

// fyi: @alexeagle

If we do that, than we can effectively discard the PR. How will dismissals be valued?

+1 that "is this approved" should always be equal to "is pullapprove green"

Let's try this setting out and just test the scenario you're concerned about? That's what we did last time we changed the logic.

#21922

"the PR XXX has not been approved by all reviewers. Ask PR owner to obtain reviews or dismiss reviews and document the dismissal reason."

👍