Skip to content

Commit

Permalink
Repo Gardening: add check for a [Type] label on PRs (#40428)
Browse files Browse the repository at this point in the history
* Repo Gardening: add check for a [Type] label on PRs

See p1HpG7-uPQ-p2#comment-74890

* Avoid an additional call to get all labels

* Move it up for more readability

See Automattic/jetpack#40428 (comment)

Co-authored-by: anomiex <anomiex@users.noreply.github.com>

---------

Co-authored-by: anomiex <anomiex@users.noreply.github.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/12142627587

Upstream-Ref: Automattic/jetpack@eafa13a
  • Loading branch information
jeherve authored and matticbot committed Dec 3, 2024
1 parent 9d91ec1 commit 0a088eb
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 39 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ This is an alpha version! The changes listed here are not final.
- Issue triage: allow querying OpenAI to get a list of labels that could potentially be added to an issue, based off the issue body, when the issue is opened.
- Issue triage: post a comment when an issue lacks labels.
- Labeling: automatically label changes to Social Previews made in the js package.
- PR checks: add new check to ensure that PRs include a [Type] label.

### Changed
- AI Labeling: allow plugin-specific feature labels as well.
- AI Labeling: clean up issue contents before we send them to OpenAI for analysis.
- AI Labeling: do not attempt to add feature labels to an issue where they were already provided
- AI Labeling: update conditions when labeling is triggered.
- Board triage: add automated triage for Photon.
- Board triage: add automatic triage to Fediverse project board.
- Board Triage: remove updateBoard task. It will now be part of the existing triageIssues task.
- Check description task: Update timing for Jetpack, wpcomsh, and mu-wpcom-plugin releases.
- Issue escalation: allow escalating the issue to multiple teams.
Expand Down
60 changes: 41 additions & 19 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48886,21 +48886,6 @@ const getLabels = __nccwpck_require__( 5479 );

/* global GitHub, WebhookPayloadPullRequest */

/**
* Check for status labels on a PR.
*
* @param {GitHub} octokit - Initialized Octokit REST client.
* @param {string} owner - Repository owner.
* @param {string} repo - Repository name.
* @param {string} number - PR number.
* @return {Promise<boolean>} Promise resolving to boolean.
*/
async function hasStatusLabels( octokit, owner, repo, number ) {
const labels = await getLabels( octokit, owner, repo, number );
// We're only interested in status labels, but not the "Needs Reply" label since it can be added by the action.
return !! labels.find( label => label.match( /^\[Status\].*(?<!Author Reply)$/ ) );
}

/**
* Check for a "Need Review" label on a PR.
*
Expand Down Expand Up @@ -49129,15 +49114,30 @@ async function getStatusChecks( payload, octokit ) {
const ownerLogin = owner.login;

const hasLongDescription = body?.length > 200;
const isLabeled = await hasStatusLabels( octokit, ownerLogin, repo, number );
const hasTesting = !! body?.includes( 'Testing instructions' );
const hasPrivacy = !! body?.includes( 'data or activity we track or use' );
const projectsWithoutChangelog = await getChangelogEntries( octokit, ownerLogin, repo, number );
const isFromContributor = head.repo.full_name === base.repo.full_name;

const prLabels = await getLabels( octokit, ownerLogin, repo, number );
const { hasStatusLabels, hasTypeLabels } = prLabels.reduce(
( acc, label ) => {
// We're only interested in status labels, but not the "Needs Reply" label since it can be added by the action.
if ( label.match( /^\[Status\].*(?<!Author Reply)$/ ) ) {
acc.hasStatusLabels = true;
}
if ( label.match( /^\[Type\]/ ) ) {
acc.hasTypeLabels = true;
}
return acc;
},
{ hasStatusLabels: false, hasTypeLabels: false }
);

return {
hasLongDescription,
isLabeled,
hasStatusLabels,
hasTypeLabels,
hasTesting,
hasPrivacy,
projectsWithoutChangelog,
Expand All @@ -49162,13 +49162,23 @@ function renderStatusChecks( statusChecks ) {
// Use labels please!
// Only check this for PRs created by a12s. External contributors cannot add labels.
if ( statusChecks.isFromContributor ) {
debug( `check-description: this PR is correctly labeled: ${ statusChecks.isLabeled }` );
debug( `check-description: this PR has a Status label: ${ statusChecks.hasStatusLabels }` );
checks += statusEntry(
! statusChecks.isLabeled,
! statusChecks.hasStatusLabels,
'Add a "[Status]" label (In Progress, Needs Team Review, ...).'
);
}

// Add a [Type] label please.
// Only check this for PRs created by a12s. External contributors cannot add labels.
if ( statusChecks.isFromContributor ) {
debug( `check-description: this PR has a Type label: ${ statusChecks.hasTypeLabels }` );
checks += statusEntry(
! statusChecks.hasTypeLabels,
'Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).'
);
}

// Check for testing instructions.
checks += statusEntry( ! statusChecks.hasTesting, 'Add testing instructions.' );

Expand Down Expand Up @@ -50739,6 +50749,18 @@ const automatticAssignments = {
slack_id: 'C048CUFRGFQ',
board_id: 'https://github.com/orgs/Automattic/projects/1106/',
},
ActivityPub: {
team: 'Fediverse',
labels: [
'[Feature] Federated comments',
'[Block] Federated reply',
'[Block] Follow Me',
'[Block] Followers',
'[Block] Post settings',
'[Block] Remote Reply',
],
board_id: 'https://github.com/orgs/Automattic/projects/1208/',
},
// Jetpack Division.
'AI Tools': {
team: 'Agora',
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

48 changes: 29 additions & 19 deletions src/tasks/check-description/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,6 @@ const getLabels = require( '../../utils/labels/get-labels' );

/* global GitHub, WebhookPayloadPullRequest */

/**
* Check for status labels on a PR.
*
* @param {GitHub} octokit - Initialized Octokit REST client.
* @param {string} owner - Repository owner.
* @param {string} repo - Repository name.
* @param {string} number - PR number.
* @return {Promise<boolean>} Promise resolving to boolean.
*/
async function hasStatusLabels( octokit, owner, repo, number ) {
const labels = await getLabels( octokit, owner, repo, number );
// We're only interested in status labels, but not the "Needs Reply" label since it can be added by the action.
return !! labels.find( label => label.match( /^\[Status\].*(?<!Author Reply)$/ ) );
}

/**
* Check for a "Need Review" label on a PR.
*
Expand Down Expand Up @@ -255,15 +240,30 @@ async function getStatusChecks( payload, octokit ) {
const ownerLogin = owner.login;

const hasLongDescription = body?.length > 200;
const isLabeled = await hasStatusLabels( octokit, ownerLogin, repo, number );
const hasTesting = !! body?.includes( 'Testing instructions' );
const hasPrivacy = !! body?.includes( 'data or activity we track or use' );
const projectsWithoutChangelog = await getChangelogEntries( octokit, ownerLogin, repo, number );
const isFromContributor = head.repo.full_name === base.repo.full_name;

const prLabels = await getLabels( octokit, ownerLogin, repo, number );
const { hasStatusLabels, hasTypeLabels } = prLabels.reduce(
( acc, label ) => {
// We're only interested in status labels, but not the "Needs Reply" label since it can be added by the action.
if ( label.match( /^\[Status\].*(?<!Author Reply)$/ ) ) {
acc.hasStatusLabels = true;
}
if ( label.match( /^\[Type\]/ ) ) {
acc.hasTypeLabels = true;
}
return acc;
},
{ hasStatusLabels: false, hasTypeLabels: false }
);

return {
hasLongDescription,
isLabeled,
hasStatusLabels,
hasTypeLabels,
hasTesting,
hasPrivacy,
projectsWithoutChangelog,
Expand All @@ -288,13 +288,23 @@ function renderStatusChecks( statusChecks ) {
// Use labels please!
// Only check this for PRs created by a12s. External contributors cannot add labels.
if ( statusChecks.isFromContributor ) {
debug( `check-description: this PR is correctly labeled: ${ statusChecks.isLabeled }` );
debug( `check-description: this PR has a Status label: ${ statusChecks.hasStatusLabels }` );
checks += statusEntry(
! statusChecks.isLabeled,
! statusChecks.hasStatusLabels,
'Add a "[Status]" label (In Progress, Needs Team Review, ...).'
);
}

// Add a [Type] label please.
// Only check this for PRs created by a12s. External contributors cannot add labels.
if ( statusChecks.isFromContributor ) {
debug( `check-description: this PR has a Type label: ${ statusChecks.hasTypeLabels }` );
checks += statusEntry(
! statusChecks.hasTypeLabels,
'Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).'
);
}

// Check for testing instructions.
checks += statusEntry( ! statusChecks.hasTesting, 'Add testing instructions.' );

Expand Down
12 changes: 12 additions & 0 deletions src/tasks/triage-issues/automattic-label-team-assignments.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ export const automatticAssignments = {
slack_id: 'C048CUFRGFQ',
board_id: 'https://github.com/orgs/Automattic/projects/1106/',
},
ActivityPub: {
team: 'Fediverse',
labels: [
'[Feature] Federated comments',
'[Block] Federated reply',
'[Block] Follow Me',
'[Block] Followers',
'[Block] Post settings',
'[Block] Remote Reply',
],
board_id: 'https://github.com/orgs/Automattic/projects/1208/',
},
// Jetpack Division.
'AI Tools': {
team: 'Agora',
Expand Down

0 comments on commit 0a088eb

Please sign in to comment.