Skip to content

Commit

Permalink
BCR PR reviewer: add support for a "doNotNotify" field
Browse files Browse the repository at this point in the history
A maintainer can request to not be notified by new PRs by setting  `"doNotNotify": true`. In this case, they can still approve the PR and have it count, but they won't be pinged.

This helps in situations like protobuf's release rotation, which has a lot of people but ideally only the releaser should be notified and be able to approve the PR. It's rather hard to support a "rotating" maintainer model, though. Thus we compromise by adding everyone from that rotation as a "do-not-notify" maintainer, and asking the releaser to watch out for the PR manually as a release step.
  • Loading branch information
Wyverald authored Sep 17, 2024
1 parent 6a8dad1 commit d0a648c
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions actions/bcr-pr-reviewer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async function fetchAllModifiedModules(octokit, owner, repo, prNumber) {
return accumulate;
}

async function generateMaintainersMap(octokit, owner, repo, modifiedModules) {
async function generateMaintainersMap(octokit, owner, repo, modifiedModules, toNotifyOnly) {
const maintainersMap = new Map(); // Map: maintainer GitHub username -> Set of module they maintain
const modulesWithoutGithubMaintainers = new Set(); // Set of module names without module maintainers
for (const moduleName of modifiedModules) {
Expand All @@ -45,7 +45,8 @@ async function generateMaintainersMap(octokit, owner, repo, modifiedModules) {
const metadata = JSON.parse(Buffer.from(metadataContent.content, 'base64').toString('utf-8'));
let hasGithubMaintainer = false;
metadata.maintainers.forEach(maintainer => {
if (maintainer.github) { // Check if the github field is specified
// Only add maintainers with a github handle set. When `toNotifyOnly`, also exclude those who have set "doNotNotify"
if (maintainer.github && !(toNotifyOnly && maintainer.doNotNotify)) {
hasGithubMaintainer = true;
if (!maintainersMap.has(maintainer.github)) {
maintainersMap.set(maintainer.github, new Set());
Expand Down Expand Up @@ -256,7 +257,7 @@ async function reviewPR(octokit, owner, repo, prNumber) {
}

// Figure out maintainers for each modified module
const [ maintainersMap, modulesWithoutGithubMaintainers ] = await generateMaintainersMap(octokit, owner, repo, modifiedModules);
const [ maintainersMap, modulesWithoutGithubMaintainers ] = await generateMaintainersMap(octokit, owner, repo, modifiedModules, /* toNotifyOnly= */ false);
console.log('Maintainers Map:');
for (const [maintainer, maintainedModules] of maintainersMap.entries()) {
console.log(`- Maintainer: ${maintainer}, Modules: ${Array.from(maintainedModules).join(', ')}`);
Expand Down Expand Up @@ -332,7 +333,7 @@ async function runNotifier(octokit) {
console.log(`Modified modules: ${Array.from(modifiedModules).join(', ')}`);

// Figure out maintainers for each modified module
const [ maintainersMap, modulesWithoutGithubMaintainers ] = await generateMaintainersMap(octokit, owner, repo, modifiedModules);
const [ maintainersMap, modulesWithoutGithubMaintainers ] = await generateMaintainersMap(octokit, owner, repo, modifiedModules, /* toNotifyOnly= */ true);

// Notify maintainers for modules with module maintainers
await notifyMaintainers(octokit, owner, repo, prNumber, maintainersMap);
Expand Down

0 comments on commit d0a648c

Please sign in to comment.