How to enforce review from codeowners without automatically requesting a review?

4.4k Views Asked by At

I have a GitHub repo which automatically requests reviews from the codeowners team (defined in .github/CODEOWNERS) when a pull request is first opened. What I want is to enforce the requirement that pull requests must be approved by a codeowner, but to stop sending these review requests when the pull request is first opened. i.e. the desired flow for a contributer is something like this:

open a pull request -> mess around, make changes -> manually request a review when ready (can't merge without codeowner approval)

This could be solved by getting contributers to open draft pull requests and only marking them as ready when they are actually ready, but contributers don't seem to want to do this. Contributers will usually open a (non-draft) pull request when it's not actually ready for review (force of habit I suppose).

Is there a way to do this, which doesn't rely on contributers using draft pull requests?

2

There are 2 best solutions below

2
On

That is very annoying, especially if you have layers of codeowners.
For instance, an ops team who has codeowner across the entire repo, but who is not reviewing most PRs day to day.

True: when you have layered code owners, with a default or catch-all team (like an ops team) that has code owner rights over the entire repository but is only supposed to review occasionally, it becomes a bit trickier.

The key challenge is to make sure the ops team does not get spammed with review requests, even if they are the catch-all codeowners, unless their review is actually needed.

You can start with fine-tuning your CODEOWNERS file.
Make sure to organize your .github/CODEOWNERS file such that specific paths and folders have their respective code owners, while the ops team is the fallback for general oversight or unassigned paths:

* @ops-team
/docs/ @documentation-team
/app/ @app-team

You will then still utilize GitHub Actions, but you will be a bit more specific this time. You will need an action that checks the files changed in the PR.
But:

  • If the files changed are specific to a team, and not in the general category of ops, then you can skip review request removal for that team and only remove for the ops team. (See "Remove requested reviewers from a pull request" API call)
  • If it touches something general, then it will automatically request a review from the ops team.

For instance:

name: Handle Review Requests

on:
  pull_request:
    types:
      - opened

jobs:
  handle-requests:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
    
    - name: Check changed files
      id: files
      run: |
        FILES=$(git diff --name-only ${{ github.event.before }} ${{ github.event.after }})
        echo "modified_files=$FILES" >> $GITHUB_ENV
    
    - name: Conditionally remove requested reviewers
      run: |
        MODIFIED_FILES="${modified_files}"
        
        # Check if the PR modifies files inside /app/ or /docs/
        if [[ "$MODIFIED_FILES" == *"app/"* ]] || [[ "$MODIFIED_FILES" == *"docs/"* ]]; then
          # Remove ops team from the review request list
          gh api \
            --method DELETE \
            /repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers \
            -F reviewers='["ops-team"]'
        fi
      env:
        GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

See also discussions/25797 for an illustration around ${{ github.event.before }} and ${{ github.event.after }}.

0
On

I found other threads on GitHub forums people asking for the same thing. I think for the moment there is no setting in GitHub where you can do this. I found this comment: "For customized workflows like this, you may want to check out Probot. The Work in Progress Probot app gives an example of how to create a bot that can block the merging of PRs based on custom logic."

I see that you can do something like this with this app

version: 2
mergeable:
  - when: pull_request.*, pull_request_review.*
    name: 'Approval check'
    validate:
      - do: approvals
        min:
          count: 1
        limit:
          users: [ 'approverA', 'approverB' ]

It might solve your problem, but for my case where there are different codeowners for different parts of the application is not good enough.