Remove BrewTestBot critical approval process.

We seem to have enough maintainers across enough timezones that this is
no longer necessary any more (and it presents a bit of a security risk).
This commit is contained in:
Mike McQuaid 2024-03-19 08:56:12 +00:00
parent e3797d39bd
commit 22bad80939
No known key found for this signature in database
2 changed files with 2 additions and 89 deletions

View File

@ -1,84 +0,0 @@
name: Triage
on:
pull_request_target:
types:
- opened
- synchronize
- reopened
- labeled
- unlabeled
permissions: {}
concurrency: triage-${{ github.head_ref }}
jobs:
review:
runs-on: ubuntu-22.04
if: startsWith(github.repository, 'Homebrew/')
steps:
- name: Review pull request
if: >
(github.event_name == 'pull_request' || github.event_name == 'pull_request_target') &&
github.event.pull_request.state != 'closed'
uses: actions/github-script@v7
with:
github-token: ${{ secrets.HOMEBREW_BREW_TRIAGE_PULL_REQUESTS_TOKEN }}
script: |
async function approvePullRequest(pullRequestNumber) {
const reviews = await approvalsByAuthenticatedUser(pullRequestNumber)
if (reviews.length > 0) {
return
}
await github.rest.pulls.createReview({
...context.repo,
pull_number: pullRequestNumber,
event: 'APPROVE',
})
}
async function approvalsByAuthenticatedUser(pullRequestNumber) {
const { data: user } = await github.rest.users.getAuthenticated()
const { data: reviews } = await github.rest.pulls.listReviews({
...context.repo,
pull_number: pullRequestNumber,
})
const approvals = reviews.filter(review => review.state == 'APPROVED')
return approvals.filter(review => review.user.login == user.login)
}
async function reviewPullRequest(pullRequestNumber) {
const { data: pullRequest } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pullRequestNumber,
})
const { data: user } = await github.rest.users.getAuthenticated()
if (pullRequest.user.login == user.login) {
core.warning('Pull request author is a bot.')
return
}
if (pullRequest.author_association != 'MEMBER') {
core.warning('Pull request author is not a member.')
return
}
const criticalLabel = 'critical'
const labels = pullRequest.labels.map(label => label.name)
const hasCriticalLabel = labels.includes(criticalLabel)
if (hasCriticalLabel) {
const message = `Review granted due to \`${criticalLabel}\` label.`
core.info(message)
await approvePullRequest(pullRequestNumber)
}
}
await reviewPullRequest(context.issue.number)

View File

@ -18,16 +18,14 @@ Merging is done using the standard "Merge" button in the `Homebrew/brew` reposit
PRs must meet the following conditions to be merged: PRs must meet the following conditions to be merged:
- Have at least one maintainer (or [@BrewTestBot](https://github.com/BrewTestBot)) approval. See the [Automatic approvals](#automatic-approvals) section below for more details about how [@BrewTestBot](https://github.com/BrewTestBot) approves PRs. - Have at least one maintainer approval.
- Have passing CI (continuous integration). This is a _mandatory_ step. PRs with failing CI should _never_ be merged. See the [CI](#ci) section below for more information about `Homebrew/brew` CI. - Have passing CI (continuous integration). This is a _mandatory_ step. PRs with failing CI should _never_ be merged. See the [CI](#ci) section below for more information about `Homebrew/brew` CI.
If possible, PRs should also have GPG-signed commits (see the private `ops` repository for instructions on setting this up). If possible, PRs should also have GPG-signed commits (see the private `ops` repository for instructions on setting this up).
### Automatic approvals ### Automatic approvals
To ensure that non-urgent PRs have the opportunity to be seen and reviewed by any other maintainers who wish to take a look, all PRs require an approval before they can be merged. However, some PRs are urgent enough that they need to be merged without an approval by another maintainer. To ensure that non-urgent PRs have the opportunity to be seen and reviewed by any other maintainers who wish to take a look, all PRs require an approval before they can be merged.
As a compromise between always needing a review and allowing maintainers to merge PRs they deem critical, the `Triage` CI job will ensure that if a PR is labelled `critical`, [@BrewTestBot](https://github.com/BrewTestBot) approves the PR, allowing it to be merged.
## CI ## CI
@ -35,7 +33,6 @@ Every PR in `Homebrew/brew` runs a series of CI tests to try to prevent bugs fro
There are many checks that run on every PR. The following is a quick list of the various checks and what they represent: There are many checks that run on every PR. The following is a quick list of the various checks and what they represent:
- `Triage / review`: This verifies that the PR has been open for long enough. See the [Automatic approvals](#automatic-approvals) section above for more information about automatic approvals.
- `Vendor Gems / vendor-gems`: This is skipped except for dependabot PRs. It updates the RBI files to match any new/changed dependencies. See [Type Checking With Sorbet](Typechecking.md) for more information about RBI files and typechecking. - `Vendor Gems / vendor-gems`: This is skipped except for dependabot PRs. It updates the RBI files to match any new/changed dependencies. See [Type Checking With Sorbet](Typechecking.md) for more information about RBI files and typechecking.
- `Codecov / codecov/patch` and `codecov/project`: These show the Codecov report for the PR. See the [`brew tests` and Codecov](#brew-tests-and-codecov) section below for more info about Codecov. - `Codecov / codecov/patch` and `codecov/project`: These show the Codecov report for the PR. See the [`brew tests` and Codecov](#brew-tests-and-codecov) section below for more info about Codecov.
- `CI / vendored gems`: This checks whether there was a change to the vendored gems on Linux that needs to be committed to the PR branch. - `CI / vendored gems`: This checks whether there was a change to the vendored gems on Linux that needs to be committed to the PR branch.