Improve some CI jobs

- make the CodeCov CI job informational. We don't want red PRs just
  because the coverage varies slightly. We still get comments inline
  saying where code coverage is met; this is more useful during review
  than a single number and failing status
- make the Triage CI job do less: instead of enforcing a time period for
  review window, make it only exist to self-approve PRs for merge and
  require a maintainer otherwise to review
This commit is contained in:
Mike McQuaid 2023-03-23 09:20:09 +00:00
parent 65d858da12
commit 510c4dce76
No known key found for this signature in database
GPG Key ID: 3338A31AFDB1D829
3 changed files with 6 additions and 135 deletions

1
.github/codecov.yml vendored
View File

@ -3,6 +3,7 @@ coverage:
status: status:
project: project:
default: default:
informational: true
threshold: 0.05% threshold: 0.05%
patch: patch:
default: default:

View File

@ -6,11 +6,8 @@ on:
- opened - opened
- synchronize - synchronize
- reopened - reopened
- closed
- labeled - labeled
- unlabeled - unlabeled
schedule:
- cron: "0 */3 * * *" # every 3 hours
permissions: {} permissions: {}
@ -21,18 +18,10 @@ jobs:
runs-on: ubuntu-22.04 runs-on: ubuntu-22.04
if: startsWith(github.repository, 'Homebrew/') if: startsWith(github.repository, 'Homebrew/')
steps: steps:
- name: Re-run this workflow
if: github.event_name == 'schedule' || github.event.action == 'closed'
uses: reitermarkus/rerun-workflow@7381e98aa2bc4464acef4b60ade8d1d1d90e6e65
with:
token: ${{ secrets.HOMEBREW_GITHUB_PUBLIC_REPO_TOKEN }}
continuous-label: waiting for feedback
trigger-labels: critical
workflow: triage.yml
- name: Review pull request - name: Review pull request
if: > if: >
(github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') &&
github.event.action != 'closed' && github.event.pull_request.state != 'closed' github.event.pull_request.state != 'closed'
uses: actions/github-script@v6 uses: actions/github-script@v6
with: with:
github-token: ${{ secrets.HOMEBREW_BREW_TRIAGE_PULL_REQUESTS_TOKEN }} github-token: ${{ secrets.HOMEBREW_BREW_TRIAGE_PULL_REQUESTS_TOKEN }}
@ -51,36 +40,6 @@ jobs:
}) })
} }
async function findComment(pullRequestNumber, id) {
const { data: comments } = await github.rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
})
const regex = new RegExp(`<!--\\s*#${id}\\s*-->`)
return comments.filter(comment => comment.body.match(regex))[0]
}
async function createOrUpdateComment(pullRequestNumber, id, message) {
const beginComment = await findComment(pullRequestNumber, id)
const body = `<!-- #${id} -->\n\n${message}`
if (beginComment) {
await github.rest.issues.updateComment({
...context.repo,
comment_id: beginComment.id,
body,
})
} else {
await github.rest.issues.createComment({
...context.repo,
issue_number: pullRequestNumber,
body,
})
}
}
async function approvalsByAuthenticatedUser(pullRequestNumber) { async function approvalsByAuthenticatedUser(pullRequestNumber) {
const { data: user } = await github.rest.users.getAuthenticated() const { data: user } = await github.rest.users.getAuthenticated()
@ -93,42 +52,6 @@ jobs:
return approvals.filter(review => review.user.login == user.login) return approvals.filter(review => review.user.login == user.login)
} }
async function dismissApprovals(pullRequestNumber, message) {
const reviews = await approvalsByAuthenticatedUser(pullRequestNumber)
for (const review of reviews) {
await github.rest.pulls.dismissReview({
...context.repo,
pull_number: pullRequestNumber,
review_id: review.id,
message: message
});
}
}
function offsetDate(start, offsetHours, skippedDays) {
let end = new Date(start)
end.setUTCHours(end.getUTCHours() + (offsetHours % 24))
while (skippedDays.includes(end.getUTCDay()) || offsetHours >= 24) {
if (!skippedDays.includes(end.getUTCDay())) {
offsetHours -= 24
}
end.setUTCDate(end.getUTCDate() + 1)
}
if (skippedDays.includes(start.getUTCDay())) {
end.setUTCHours(offsetHours, 0, 0)
}
return end
}
function formatDate(date) {
return date.toISOString().replace(/\.\d+Z$/, ' UTC').replace('T', ' at ')
}
async function reviewPullRequest(pullRequestNumber) { async function reviewPullRequest(pullRequestNumber) {
const { data: pullRequest } = await github.rest.pulls.get({ const { data: pullRequest } = await github.rest.pulls.get({
owner: context.repo.owner, owner: context.repo.owner,
@ -147,65 +70,14 @@ jobs:
return return
} }
const reviewLabel = 'waiting for feedback'
const criticalLabel = 'critical' const criticalLabel = 'critical'
const labels = pullRequest.labels.map(label => label.name) const labels = pullRequest.labels.map(label => label.name)
const hasReviewLabel = labels.includes(reviewLabel)
const hasCriticalLabel = labels.includes(criticalLabel) const hasCriticalLabel = labels.includes(criticalLabel)
const offsetHours = 24 if (hasCriticalLabel) {
const skippedDays = [ const message = `Review granted due to \`${criticalLabel}\` label.`
6, // Saturday
0, // Sunday
]
const currentDate = new Date()
const reviewStartDate = new Date(pullRequest.created_at)
const reviewEndDate = offsetDate(reviewStartDate, offsetHours, skippedDays)
const reviewEnded = currentDate > reviewEndDate
if (reviewEnded || hasCriticalLabel) {
let message
if (hasCriticalLabel && !reviewEnded) {
message = `Review period skipped due to \`${criticalLabel}\` label.`
} else {
message = 'Review period ended.'
}
if (hasReviewLabel) {
await github.rest.issues.removeLabel({
...context.repo,
issue_number: pullRequestNumber,
name: reviewLabel,
})
}
core.info(message) core.info(message)
await createOrUpdateComment(pullRequestNumber, 'review-period-end', message)
await approvePullRequest(pullRequestNumber) await approvePullRequest(pullRequestNumber)
} else {
const message = `Review period will end on ${formatDate(reviewEndDate)}.`
core.info(message)
await dismissApprovals(pullRequestNumber, 'Review period has not ended yet.')
await createOrUpdateComment(pullRequestNumber, 'review-period-begin', message)
const endComment = await findComment(pullRequestNumber, 'review-period-end')
if (endComment) {
await github.rest.issues.deleteComment({
...context.repo,
comment_id: endComment.id,
})
}
await github.rest.issues.addLabels({
...context.repo,
issue_number: pullRequestNumber,
labels: [reviewLabel],
})
core.setFailed('Review period has not ended yet.')
} }
} }

View File

@ -25,11 +25,9 @@ If possible, PRs should also have GPG-signed commits (see the private `ops` repo
### 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, not every PR is reviewed by another maintainer, and 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. However, some PRs are urgent enough that they need to be merged without an approval by another maintainer.
As a compromise between always needing a review and allowing maintainers to merge PRs they deem ready, the `Triage` CI job will ensure that PRs cannot be merged until they've been open for 24 hours (only counting hours that occur Monday to Friday). After the triage period has expired, the CI job will show up as "passed" and [@BrewTestBot](https://github.com/BrewTestBot) will approve the PR, allowing it to be merged. This gives all maintainers a reasonable opportunity to review every PR, but won't block any PR for lack of reviews. 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.
If the PR is urgent enough that it is necessary to bypass that 24 hour window, the `critical` label should be applied to the PR. When this label is applied, the `Triage` CI job will immediately be successful and [@BrewTestBot](https://github.com/BrewTestBot) will approve the PR.
## CI ## CI