Skip to content

Commit

Permalink
BCR reviewer: Add diff_module action type (#2158)
Browse files Browse the repository at this point in the history
This action will diff new module versions in a BCR PR against its
previous version. Making it much easier to review changes between module
versions.

Test runs:
-
https://github.com/meteorcloudy/bazel-central-registry/actions/runs/12792158335/job/35663185312?pr=21
-
https://github.com/meteorcloudy/bazel-central-registry/actions/runs/12792705316/job/35663784300?pr=21
  • Loading branch information
meteorcloudy authored Jan 17, 2025
1 parent b8da1b8 commit abb92bb
Showing 1 changed file with 88 additions and 6 deletions.
94 changes: 88 additions & 6 deletions actions/bcr-pr-reviewer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ async function fetchAllModifiedModules(octokit, owner, repo, prNumber) {
});

response.data.forEach(file => {
const match = file.filename.match(/^modules\/([^\/]+)\//);
const match = file.filename.match(/^modules\/([^\/]+)\/([^\/]+)\//);
if (match) {
accumulate.add(match[1]);
accumulate.add(`${match[1]}@${match[2]}`);
}
});

Expand Down Expand Up @@ -108,7 +108,8 @@ async function notifyMaintainers(octokit, owner, repo, prNumber, maintainersMap)
}
const maintainersList = Array.from(maintainersCopy).join(', ');
console.log(`Notifying ${maintainersList} for modules: ${modulesList}`);
const commentBody = `Hello ${maintainersList}, modules you maintain (${modulesList}) have been updated in this PR. Please review the changes.`;
const commentBody = `Hello ${maintainersList}, modules you maintain (${modulesList}) have been updated in this PR.
Please review the changes. You can view a diff against the previous version in the "Generate module diff" check.`;
await postComment(octokit, owner, repo, prNumber, commentBody);
}
}
Expand Down Expand Up @@ -249,7 +250,8 @@ async function reviewPR(octokit, owner, repo, prNumber) {
}

// Fetch modified modules
const modifiedModules = await fetchAllModifiedModules(octokit, owner, repo, prNumber);
const modifiedModuleVersions = await fetchAllModifiedModules(octokit, owner, repo, prNumber);
const modifiedModules = new Set(Array.from(modifiedModuleVersions).map(module => module.split('@')[0]));
console.log(`Modified modules: ${Array.from(modifiedModules).join(', ')}`);
if (modifiedModules.size === 0) {
console.log('No modules are modified in this PR');
Expand Down Expand Up @@ -329,7 +331,8 @@ async function runNotifier(octokit) {
const { owner, repo } = context.repo;

// Fetch modified modules
const modifiedModules = await fetchAllModifiedModules(octokit, owner, repo, prNumber);
const modifiedModuleVersions = await fetchAllModifiedModules(octokit, owner, repo, prNumber);
const modifiedModules = new Set(Array.from(modifiedModuleVersions).map(module => module.split('@')[0]));
console.log(`Modified modules: ${Array.from(modifiedModules).join(', ')}`);

// Figure out maintainers for each modified module
Expand All @@ -342,7 +345,9 @@ async function runNotifier(octokit) {
if (modulesWithoutGithubMaintainers.size > 0) {
const modulesList = Array.from(modulesWithoutGithubMaintainers).join(', ');
console.log(`Notifying @bazelbuild/bcr-maintainers for modules: ${modulesList}`);
await postComment(octokit, owner, repo, prNumber, `Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (${modulesList}) have been updated in this PR. Please review the changes.`);
await postComment(octokit, owner, repo, prNumber,
`Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (${modulesList}) have been updated in this PR.
Please review the changes. You can view a diff against the previous version in the "Generate module diff" check.`);
}
}

Expand Down Expand Up @@ -477,6 +482,81 @@ async function runSkipCheck(octokit) {
}
}

async function runDiffModule(octokit) {
const prNumber = context.issue.number;
if (!prNumber) {
console.log('Could not get pull request number from context, exiting');
return;
}
console.log(`Diffing modules in PR #${prNumber}`);

const { owner, repo } = context.repo;

// Fetch modified modules
const modifiedModuleVersions = await fetchAllModifiedModules(octokit, owner, repo, prNumber);
console.log(`Modified modules: ${Array.from(modifiedModuleVersions).join(', ')}`);

// Use group if more than one module are modified
const groupStart = modifiedModuleVersions.size === 1 ? "" : "::group::";
const groupEnd = modifiedModuleVersions.size === 1 ? "" : "::endgroup::";

for (const moduleVersion of modifiedModuleVersions) {
const [moduleName, versionName] = moduleVersion.split('@');
try {
const { data: metadataContent } = await octokit.rest.repos.getContent({
owner,
repo,
path: `modules/${moduleName}/metadata.json`,
ref: `refs/pull/${prNumber}/head`,
});
const metadata = JSON.parse(Buffer.from(metadataContent.content, 'base64').toString('utf-8'));

// Assuming metadata.versions is sorted in ascending order, otherwise bcr_validation.py checks will fail anyway.
let versionIndex = metadata.versions.findIndex(version => version === versionName);

if (versionIndex === -1) {
console.error(`Version ${versionName} not found in metadata.json for module ${moduleName}`);
console.log(`Metadata content for module ${moduleName}@${versionName}: ${JSON.stringify(metadata, null, 2)}`);
setFailed(`Failed to generate diff for module ${moduleName}@${versionName}`);
return;
}

if (versionIndex === 0) {
console.log(`No previous version to diff for module ${moduleName}@${versionName}`);
continue;
}

const previousVersion = metadata.versions[versionIndex - 1];
console.log(`${groupStart}Generating diff for module ${moduleName}@${versionName} against version ${previousVersion}`);

const diffCommand = `diff --color=always -urN modules/${moduleName}/${previousVersion} modules/${moduleName}/${versionName}`;
console.log(`Running command: ${diffCommand}`);
const { execSync } = require('child_process');

try {
await execSync(diffCommand, { stdio: 'inherit' });
console.log(`No differences found!`);
} catch (error) {
if (error.status === 1) {
// diff command returns 1 when differences are found
} else {
setFailed(`Failed to generate diff for module ${moduleName}@${versionName}`);
throw error;
}
}

console.log(`${groupEnd}`);
} catch (error) {
if (error.status === 404) {
console.log(`Module ${moduleName} does not have a metadata.json file on the PR branch.`);
} else {
console.error(`Error processing module ${moduleName}: ${error}`);
setFailed(`Failed to generate diff for module ${moduleName}@${versionName}`);
}
}
}
}

async function run() {
const action_type = getInput("action-type");
const token = getInput("token");
Expand All @@ -490,6 +570,8 @@ async function run() {
await runDismissApproval(octokit);
} else if (action_type === "skip_check") {
await runSkipCheck(octokit);
} else if (action_type === "diff_module") {
await runDiffModule(octokit);
} else {
console.log(`Unknown action type: ${action_type}`);
}
Expand Down

0 comments on commit abb92bb

Please sign in to comment.