Skip to content

Commit

Permalink
Overhaul the complete labelling system (#4438)
Browse files Browse the repository at this point in the history
* Record change info during PR automation init

* Place change info next to event info for workflow to read

* Replace `getLabel` with `getAllLabels` to reduce API calls

* Transfer `auto-label.json` patterns to `filters.yml`

* Update comments and text

* Integrate labelling for stack and other path-based labels

* Drop `banyan/auto-label`

* Remove labelling job from CI + CD workflow
  • Loading branch information
dhruvkb committed Jun 6, 2024
1 parent b76474e commit e3820fe
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 89 deletions.
7 changes: 0 additions & 7 deletions .github/auto-label.json

This file was deleted.

6 changes: 6 additions & 0 deletions .github/filters.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,9 @@ mgmt:
ci_cd:
- .github/actions/**
- .github/workflows/ci_cd.yml
migrations:
- api/api/migrations/**
project_proposal:
- documentation/projects/**/*project_proposal*
project_ip:
- documentation/projects/**/*implementation_plan*
44 changes: 0 additions & 44 deletions .github/workflows/ci_cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,50 +139,6 @@ jobs:
checks: "files,duppatterns,syntax"
experimental_checks: "notowned,avoid-shadowing"

add-stack-label:
name: Add stack label
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.owner.login == 'WordPress' &&
github.actor != 'dependabot[bot]'
runs-on: ubuntu-latest
needs:
- get-changes

steps:
- name: Apply stack labels
uses: actions/github-script@v7
with:
# Also see
# - list of stack filters: `.github/filters.yml`
# - list of stack labels: https://github.com/WordPress/openverse/labels?q=stack
script: |
const labels = await github.rest.issues.listLabelsOnIssue({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
})
console.log(labels.data)
if (labels.data.some(label => label.name.startsWith("🧱 stack: "))) {
console.log("Stack label already applied, skipping.")
} else {
const stacks = ["catalog", "api", "ingestion_server", "frontend", "documentation", "mgmt"]
const labels = JSON
.parse('${{ needs.get-changes.outputs.changes }}')
.filter(change => stacks.includes(change))
.map(change => `🧱 stack: ${change.replace("_", " ")}`)
if (!labels.length) {
console.log("Couldn't determine stack, applying triage labels.")
labels.push("🚦 status: awaiting triage", "🏷 status: label work required")
}
await github.rest.issues.addLabels({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
labels,
})
}
build-images:
name: Build Docker images
if: needs.determine-images.outputs.do_build == 'true'
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/migration_safety_warning.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,3 @@ jobs:
issue-number: ${{ github.event.pull_request.number }}
body: |
This PR has migrations. Please rebase it before merging to ensure that conflicting migrations are not introduced.
- uses: banyan/auto-label@1.2
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
31 changes: 20 additions & 11 deletions .github/workflows/pr_automations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,27 +45,36 @@ jobs:
uses: actions/github-script@v7
with:
script: |
let fs = require('fs');
let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: context.payload.workflow_run.id,
});
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
return artifact.name == "event_info"
})[0];
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
let matchArtifacts = allArtifacts.data.artifacts.filter((artifact) => {
return ["event_info", "change_info"].includes(artifact.name)
});
let fs = require('fs');
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/event_info.zip`, Buffer.from(download.data));
for (let matchArtifact of matchArtifacts)
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
fs.writeFileSync(
`${process.env.GITHUB_WORKSPACE}/{matchArtifact.name}.zip`,
Buffer.from(download.data)
);
);
- name: Unzip artifact
- name: Unzip artifacts
run: |
unzip event_info.zip
mv event.json /tmp/event.json
cat /tmp/event.json
unzip change_info.zip
mv change.json /tmp/change.json
cat /tmp/change.json
- name: Perform PR labelling
uses: actions/github-script@v7
Expand Down
29 changes: 28 additions & 1 deletion .github/workflows/pr_automations_init.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,34 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.node_id }}

jobs:
run:
change-info:
name: Save change info
runs-on: ubuntu-latest
permissions:
pull-requests: read
# Prevent running this workflow on forks, it's unnecessary for external contributors
if: github.repository_owner == 'WordPress'
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Get changes
id: paths-filter
uses: ./.github/actions/get-changes

- name: Save change info
run: |
echo "$CHANGES" > /tmp/change.json
env:
CHANGES: ${{ steps.paths-filter.outputs.changes }}

- name: Upload change info as artifact
uses: actions/upload-artifact@v4
with:
name: change_info
path: /tmp/change.json

event-info:
name: Save event info
runs-on: ubuntu-latest
# Prevent running this workflow on forks, it's unnecessary for external contributors
Expand Down
81 changes: 59 additions & 22 deletions automations/js/src/label_pr.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,35 @@ import { PullRequest } from './utils/pr.mjs'
import { IdSet } from './utils/id_set.mjs'

const exactlyOne = ['priority', 'goal']
const atleastOne = ['aspect']
const atleastOneCheckOnly = ['stack']
const atleastOne = ['aspect', 'stack']

const pathLabels = {
migrations: 'migrations',
project_proposal: '🧭 project: proposal',
project_ip: '🧭 project: implementation plan',
}

/**
* Get the list of labels that are applicable to a PR based on the changes.
*
* @param allLabels {import('./utils/pr.mjs').Label[]} list of all labels in the repo
* @param changes {string[]} the list of groups that are changed by the PR
* @returns {import('./utils/pr.mjs').Label[]} the labels for the PR based on changes
*/
function getLabelsFromChanges(allLabels, changes) {
const applicableLabels = allLabels
.filter((label) => label.name.startsWith('🧱 stack:'))
.filter((label) => {
const [, stackName] = label.name.split(': ')
return changes.includes(stackName)
})
Object.entries(pathLabels).forEach(([group, labelName]) => {
if (changes.includes(group)) {
applicableLabels.push(allLabels.find((label) => label.name === labelName))
}
})
return applicableLabels
}

/**
* Check if the list of labels covers all requirements.
Expand All @@ -18,7 +45,7 @@ function getIsFullyLabeled(labels) {
return false
}
}
for (let req of atleastOne + atleastOneCheckOnly) {
for (let req of atleastOne) {
if (labels.filter((label) => label.name.includes(req)).length < 1) {
return false
}
Expand All @@ -27,28 +54,28 @@ function getIsFullyLabeled(labels) {
}

/**
* Get the `Label` instance from a label's name.
* Get all `Label` instances for a repository.
*
* @param octokit {import('@octokit/rest').Octokit} the Octokit instance to use
* @param repository {string} the full name of the repository, including owner
* @param name {string} the name of the label for which to get node ID
* @returns {Label} the label with the `id` and `name` fields
* @returns {import('./utils/pr.mjs').Label[]} the label with the `id` and `name` fields
*/
async function getLabel(octokit, repository, name) {
async function getAllLabels(octokit, repository) {
const [owner, repo] = repository.split('/')
const res = await octokit.rest.issues.getLabel({ owner, repo, name })
return {
id: res.data.node_id,
name,
}
const res = await octokit.rest.issues.listLabelsForRepo({
owner,
repo,
per_page: 100,
})
return res.data.map((item) => ({
id: item.node_id,
name: item.name,
}))
}

/**
* Apply labels to a PR based on the PR's linked issues.
*
* Note that this function does not concern itself with the management of stack
* labels as that is performed by a job in the CI + CD workflow.
*
* @param octokit {import('@octokit/rest').Octokit} the Octokit instance to use
* @param core {import('@actions/core')} GitHub Actions toolkit, for logging
*/
Expand All @@ -57,6 +84,9 @@ export const main = async (octokit, core) => {
const { eventName, eventAction, prNodeId } = JSON.parse(
readFileSync('/tmp/event.json', 'utf-8')
)
const changes = JSON.parse(readFileSync('/tmp/change.json', 'utf-8'))

const allLabels = await getAllLabels(octokit, GITHUB_REPOSITORY)

if (
eventName !== 'pull_request' ||
Expand All @@ -72,19 +102,26 @@ export const main = async (octokit, core) => {
await pr.init()

let isTriaged = false
if (pr.labels && pr.labels.some((label) => !label.name.includes('stack'))) {
// If a PR has non-stack labels, it has likely been triaged by a maintainer.
core.info('The PR already has non-stack labels.')
if (pr.labels.length) {
// If a PR already has some labels, it is considered triaged.
core.info('The PR already has some labels.')
isTriaged = true
}

// The logic for labelling a PR is as follows.
const finalLabels = new IdSet()

// We start with the PRs current labels. We do not remove any labels already
// set as they could be the work of the CI labeller job or a maintainer.
// set as they could be the work of a maintainer.
pr.labels.forEach((label) => {
core.debug(`Adding label "${label.name}" from PR.`)
core.debug(`Retaining label "${label.name}" from PR.`)
finalLabels.add(label)
})

// Here we determine the labels from the groups changed by the PR. This
// consists of stack labels and some very specific labels based on file paths.
getLabelsFromChanges(allLabels, changes).forEach((label) => {
core.info(`Adding change-based label "${label.name}" to PR.`)
finalLabels.add(label)
})

Expand Down Expand Up @@ -131,8 +168,8 @@ export const main = async (octokit, core) => {
} else {
attnLabel = '🚦 status: awaiting triage'
}
core.info(`Pull not fully labelled so adding "${attnLabel}".`)
attnLabel = await getLabel(octokit, GITHUB_REPOSITORY, attnLabel)
core.info(`PR not fully labelled so adding "${attnLabel}".`)
attnLabel = allLabels.filter((item) => item.name === attnLabel)[0]
finalLabels.add(attnLabel)
}

Expand Down

0 comments on commit e3820fe

Please sign in to comment.