diff --git a/.github/policies/prs.external-reviewers.generated.yml b/.github/policies/prs.external-reviewers.generated.yml new file mode 100644 index 00000000000..404ceb92720 --- /dev/null +++ b/.github/policies/prs.external-reviewers.generated.yml @@ -0,0 +1,32 @@ +# This file is generated by the sync-labels script DO NOT EDIT manually +# cspell:ignore fionabronwen steverice swatkatz +id: prs.external-reviewers +name: Notify external area owners on PRs +description: GitHub doesn't allow CODEOWNERS entries or formal review requests + for users that are not repository collaborators, but some areas are + contributed by external teams. Mention those owners in a comment instead. + https://github.com/orgs/community/discussions/23042 +resource: repository +disabled: false +configuration: + resourceManagementConfiguration: + eventResponderTasks: + - if: + - payloadType: Pull_Request + then: + - if: + - includesModifiedFiles: + files: + - packages/graphql/**/* + - not: + hasLabel: + label: emitter:graphql + then: + - addReply: + reply: "@fionabronwen @swatkatz @steverice — this PR modifies files in the + `emitter:graphql` area, which your team owns. Please take a + look. (You can't be added as a formal reviewer because + you're not a repository collaborator, so this is a heads-up + instead.)" + - addLabel: + label: emitter:graphql diff --git a/.github/policies/prs.external-reviewers.yml b/.github/policies/prs.external-reviewers.yml deleted file mode 100644 index 697150e62a5..00000000000 --- a/.github/policies/prs.external-reviewers.yml +++ /dev/null @@ -1,21 +0,0 @@ -# cspell:ignore swatkatz -id: prs.external-reviewers -name: Assign External Reviewers to PRs. -description: Github doesn't allow CODEOWNERS to have external users but some emitters are contributed externally https://github.com/orgs/community/discussions/23042 -resource: repository -disabled: false -configuration: - resourceManagementConfiguration: - eventResponderTasks: - - if: - - payloadType: Pull_Request - then: - - if: - - includesModifiedFiles: - files: - - packages/graphql/**/* - then: - - requestReview: - reviewer: steverice - - requestReview: - reviewer: swatkatz diff --git a/eng/common/config/area.ts b/eng/common/config/area.ts index d66a60b59de..f1c12ec9726 100644 --- a/eng/common/config/area.ts +++ b/eng/common/config/area.ts @@ -34,6 +34,18 @@ export const AreaPaths: Record = { spector: ["packages/spector/", "packages/http-specs"], }; +/** + * External area owners: teams/users outside the org that should be notified on PRs + * touching their area. GitHub does not allow non-collaborators in CODEOWNERS, nor can + * they be added as formal reviewers, so they are @-mentioned in a comment instead. + * + * Keyed by the same area names as {@link AreaPaths}; the paths are reused from there. + */ +export const ExternalOwners: Partial> = { + // cspell:ignore swatkatz fionabronwen + "emitter:graphql": ["fionabronwen", "swatkatz", "steverice"], +}; + /** * Path that should trigger every CI build. */ diff --git a/eng/common/config/labels.ts b/eng/common/config/labels.ts index 286c297f5f5..24a8f197ae4 100644 --- a/eng/common/config/labels.ts +++ b/eng/common/config/labels.ts @@ -1,7 +1,7 @@ // cspell:ignore bfff import { defineConfig, defineLabels } from "../scripts/labels/config.js"; import { repo } from "../scripts/utils/common.js"; -import { AreaPaths } from "./area.js"; +import { AreaPaths, ExternalOwners } from "./area.js"; /** * Labels that are used to categorize issue for which area they belong to. @@ -230,4 +230,5 @@ export default defineConfig({ }, }, areaPaths: AreaPaths, + externalOwners: ExternalOwners, }); diff --git a/eng/common/scripts/labels/automation.ts b/eng/common/scripts/labels/automation.ts index f3b110cc2ee..da0d1cac0a4 100644 --- a/eng/common/scripts/labels/automation.ts +++ b/eng/common/scripts/labels/automation.ts @@ -4,6 +4,8 @@ import { CheckOptions, syncFile } from "../utils/common.js"; import { expandFolder } from "../utils/find-area-changed.js"; import { PolicyServiceConfig, + addLabel, + addReply, and, eventResponderTask, hasLabel, @@ -24,15 +26,35 @@ export interface SyncLabelAutomationOptions extends CheckOptions {} export async function syncLabelAutomation(config: RepoConfig, options: SyncLabelAutomationOptions) { await syncPolicyFile(createIssueTriageConfig(config), options); await syncPolicyFile(createPrTriageConfig(config), options); + await syncPolicyFile( + createExternalReviewersConfig(config), + options, + collectExternalOwners(config), + ); } -async function syncPolicyFile(policy: PolicyServiceConfig, options: CheckOptions) { +async function syncPolicyFile( + policy: PolicyServiceConfig, + options: CheckOptions, + ignoreWords: string[] = [], +) { const yaml = stringify(policy); - const content = `# This file is generated by the sync-labels script DO NOT EDIT manually\n${yaml}`; + const cspell = ignoreWords.length > 0 ? `# cspell:ignore ${ignoreWords.join(" ")}\n` : ""; + const content = `# This file is generated by the sync-labels script DO NOT EDIT manually\n${cspell}${yaml}`; const filename = resolve(policyFolder, `${policy.id}.generated.yml`); await syncFile(filename, content, options); } +function collectExternalOwners(config: RepoConfig): string[] { + const all = new Set(); + for (const owners of Object.values(config.externalOwners)) { + for (const owner of owners) { + all.add(owner); + } + } + return [...all].sort(); +} + function createIssueTriageConfig(config: RepoConfig): PolicyServiceConfig { const areaLabels = config.labels.area.labels; return { @@ -129,3 +151,40 @@ function createPrTriageConfig(config: RepoConfig): PolicyServiceConfig { }, }; } +function createExternalReviewersConfig(config: RepoConfig): PolicyServiceConfig { + const entries = Object.entries(config.externalOwners).filter(([, owners]) => owners.length > 0); + return { + id: "prs.external-reviewers", + name: "Notify external area owners on PRs", + description: + "GitHub doesn't allow CODEOWNERS entries or formal review requests for users that are not repository collaborators, but some areas are contributed by external teams. Mention those owners in a comment instead. https://github.com/orgs/community/discussions/23042", + resource: "repository", + disabled: false, + configuration: { + resourceManagementConfiguration: { + eventResponderTasks: [ + eventResponderTask({ + if: [payloadType("Pull_Request")], + then: entries.map(([area, owners]) => { + const globs = (config.areaPaths[area] ?? []).map(expandFolder); + const mentions = owners.map((owner) => `@${owner}`).join(" "); + // The area label doubles as the "already notified" guard: it is added + // below the first time the paths match so owners are pinged once and not + // on every subsequent push (the prs.triage policy also adds it, but adding + // it here guarantees the guard is set even if that policy hasn't run yet). + return { + if: [includesModifiedFiles(globs), not(hasLabel(area))], + then: [ + addReply( + `${mentions} — this PR modifies files in the \`${area}\` area, which your team owns. Please take a look. (You can't be added as a formal reviewer because you're not a repository collaborator, so this is a heads-up instead.)`, + ), + addLabel(area), + ], + }; + }), + }), + ], + }, + }, + }; +} diff --git a/eng/common/scripts/labels/policy.ts b/eng/common/scripts/labels/policy.ts index fb04c9b6348..df2307d368f 100644 --- a/eng/common/scripts/labels/policy.ts +++ b/eng/common/scripts/labels/policy.ts @@ -166,3 +166,23 @@ export type EventResponderTask = { export function eventResponderTask(options: EventResponderTask): EventResponderTask { return options; } + +export type AddReply = { + addReply: { reply: string }; +}; + +/** + * Post a comment on the issue/PR. @-mentions in the body notify the mentioned users, + * including users that are not repository collaborators. + */ +export function addReply(reply: string): AddReply { + return { addReply: { reply } }; +} + +export type AddLabel = { + addLabel: { label: string }; +}; + +export function addLabel(label: string): AddLabel { + return { addLabel: { label } }; +} diff --git a/eng/common/scripts/labels/types.ts b/eng/common/scripts/labels/types.ts index a461d553d12..4c1809f6d71 100644 --- a/eng/common/scripts/labels/types.ts +++ b/eng/common/scripts/labels/types.ts @@ -5,6 +5,7 @@ export interface RepoConfig { }; labels: LabelsConfig; areaPaths: Record; + externalOwners: Record; } export interface LabelsConfig {