Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve some CI jobs #15039

Merged
merged 1 commit into from Mar 23, 2023
Merged

Improve some CI jobs #15039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/codecov.yml
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ coverage:
status:
project:
default:
informational: true
threshold: 0.05%
patch:
default:
Expand Down
134 changes: 3 additions & 131 deletions .github/workflows/triage.yml
View file
Open in desktop
Expand Up @@ -6,11 +6,8 @@ on:
- opened
- synchronize
- reopened
- closed
- labeled
- unlabeled
schedule:
- cron: "0 */3 * * *" # every 3 hours

permissions: {}

Expand All @@ -21,18 +18,10 @@ jobs:
runs-on: ubuntu-22.04
if: startsWith(github.repository, 'Homebrew/')
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
if: >
(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
with:
github-token: ${{ secrets.HOMEBREW_BREW_TRIAGE_PULL_REQUESTS_TOKEN }}
Expand All @@ -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) {
const { data: user } = await github.rest.users.getAuthenticated()

Expand All @@ -93,42 +52,6 @@ jobs:
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) {
const { data: pullRequest } = await github.rest.pulls.get({
owner: context.repo.owner,
Expand All @@ -147,65 +70,14 @@ jobs:
return
}

const reviewLabel = 'waiting for feedback'
const criticalLabel = 'critical'

const labels = pullRequest.labels.map(label => label.name)
const hasReviewLabel = labels.includes(reviewLabel)
const hasCriticalLabel = labels.includes(criticalLabel)

const offsetHours = 24
const skippedDays = [
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,
})
}

if (hasCriticalLabel) {
const message = `Review granted due to \`${criticalLabel}\` label.`
core.info(message)
await createOrUpdateComment(pullRequestNumber, 'review-period-end', message)
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.')
}
}

Expand Down
6 changes: 2 additions & 4 deletions docs/Homebrew-brew-Maintainer-Guide.md
View file
Open in desktop
Expand Up @@ -25,11 +25,9 @@ If possible, PRs should also have GPG-signed commits (see the private `ops` repo

### 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.

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.
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

Expand Down