mergeable icon indicating copy to clipboard operation
mergeable copied to clipboard

When author of a PR is required as a reviewer, the PR is blocked

Open vandac opened this issue 4 years ago • 3 comments

`mergeable:

  • when: pull_request., pull_request_review. name: '2approvals and 1 plus2' validate:
    • do: approvals min: count: 2 message: 'We require at least 2 approvals.' or:
      • required: reviewers: [ plus2reviewer ]`

In the configuration above, we specify that our PRs require at least two approvals and additionally, one approval must be granted by one of the named reviewers. If there is only one reviewer and the same reviewer is also an author of a given Pull request, the Pull Request is blocked from merging. The author should be removed from the required approvals as the author cannot approve their own PRs.

vandac avatar Aug 24 '21 15:08 vandac

@vandac Hi, first of all, your configuration looks off from your description of the use case. ~~re author as reviewer: I believe we are already dismissing author as a reviewer so if the author approve, we just simply don't count it as an approval.~~

 - when: pull_request., pull_request_review.
    name: '2approvals and 1 plus2'
    validate:
    - do: approvals
      and: 
        - min:
          count: 2
          message: 'We require at least 2 approvals.'
        - required:
           reviewers: [ plus2reviewer ]

(Note the indentation may be off) This config seems more closely aligned with your use case where you require 2 approval minimum AND one of them must be a named approver.

EDIT: I just checked the code, I was wrong about dismissing author as a reviewer

EDIT 2: I just re-read your comment and I think filter could be used as a stop gap to support your use case. you'll need to have two recipe, one with filter: must_include required author and filter: must_exclude required author

 - when: pull_request., pull_request_review.
    name: '2approvals and 1 plus2'
    filter: // only when the required reviewer is not the author
    validate:
    - do: approvals
      and: 
        - min:
          count: 2
          message: 'We require at least 2 approvals.'
        - required:
           reviewers: [ plus2reviewer ]

shine2lay avatar Aug 24 '21 16:08 shine2lay

Hello, thank you for your answer. What I only don't understand is why wo recipes are needed. If I always want to exclude the author from required reviewers, don't I need just the second filter?

vandac avatar Aug 27 '21 13:08 vandac

@vandac if you only have one recipe where filter: must_exclude required author, the validation suite will not run at all for when the author is the required author. If you are okay with it, then i guess 1 recipe is fine.

shine2lay avatar Aug 29 '21 19:08 shine2lay