Mergifyio / mergify

Mergify Community Issue Tracker
https://mergify.com
Apache License 2.0
318 stars 91 forks source link

`request_reviews` triggers in excess for a team when a member of the team approves a pull request #5109

Closed mkatychev closed 4 months ago

mkatychev commented 4 months ago

Expected Behavior

If a member of an team responds or appros so that the review request of said team is dropped, the request review form team does not get reapplied

Actual Behavior

image

Steps to Reproduce the Problem

  1. have a PR that meets the above conditions
  2. PR is approved by a member of the team(s) under actions

Specifications

# .mergify/config.yml
pull_request_rules:
  - name: add infra to PRs that touch deployment files
    # https://docs.mergify.com/conditions/#how-to-match-lists
    conditions:
      - label=infra
      - -draft
      - -closed
      - base=main
    actions:
      request_reviews:
        teams:
          - "@team/infra"
DouglasBlackwood commented 4 months ago

Hi!

Mergify will request the review until the conditions are met. To avoid requesting indefinitely, you could add -approved-reviews-by=@team/infra in the conditions, so Mergify will request the review only if @team/infra didn't approve.

# .mergify/config.yml
pull_request_rules:
  - name: add infra to PRs that touch deployment files
    # https://docs.mergify.com/conditions/#how-to-match-lists
    conditions:
      - label=infra
      - -draft
      - -closed
      - base=main
      - -approved-reviews-by=@team/infra
    actions:
      request_reviews:
        teams:
          - "@team/infra"
mkatychev commented 4 months ago

Thanks for the reply @DouglasBlackwood , this scenario also re-triggers when a member of the team comments: image And in that case you would have to do the same and ensure something like a user in users_from_teams didn't comment? If I were to apply something akin to:

conditions:
  - label=infra
  - -draft
  - -closed
  - base=main
  - -approved-reviews-by=@team/infra
  - -user-comments-from=@team/infra

...then it feels that one needs multiple workarounds to get the actions.request_reviews.teams feature to work in a predictable manner.

If this is working as intended then I would prefer to hardcode individual team members and wait for additional functionality to be added to actions.request_reviews.teams because none of the examples currently shown on docs.mergify.com imply that review requests fire each time a team member does an action that allows anyone to still request that team for review.

DouglasBlackwood commented 4 months ago

In your example, infra-member reviewed on behalf of team/infra. So Mergify sees that there is no review requested anymore and triggers again. You could tell Mergify to not request a review until all threads are solved using #changes-requested-reviews-by=0.

So your configuration could look like this:

conditions:
  - label=infra
  - -draft
  - -closed
  - base=main
  - -approved-reviews-by=@team/infra
  - "#changes-requested-reviews-by=0"
mkatychev commented 4 months ago

thanks for the update!