Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
unify the logging. need to make certain we're not too aggressive with…
… the setting of Automated Requirements met, and to do that we need build history
  • Loading branch information
scbedd committed Jul 23, 2025
commit a325324d9e4300da68a3c0c39a75c77685f0b9c0
77 changes: 43 additions & 34 deletions .github/workflows/src/summarize-checks/summarize-checks.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// @ts-check

/*
Expand Down Expand Up @@ -319,9 +319,7 @@
event_name,
targetBranch,
) {
core.info(
`Handling ${event_name} event for PR #${issue_number} in ${owner}/${repo} with targeted branch ${targetBranch}`,
);
core.info(`Handling ${event_name} event for PR #${issue_number} in ${owner}/${repo}.`);

// retrieve latest labels state
const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
Expand All @@ -345,31 +343,33 @@
/** @type {string[]} */
let labelNames = labels.map((/** @type {{ name: string; }} */ label) => label.name);

// todo: how important is it that we know if we're in draft? I don't want to have to pull the PR details unless we actually need to
// do we need to pull this from the PR? if triggered from a workflow_run we won't have payload.pullrequest populated
let labelContext = await updateLabels(labelNames, impactAssessment);

for (const label of labelContext.toRemove) {
core.info(`Removing label: ${label} from ${owner}/${repo}#${issue_number}.`);
// await github.rest.issues.removeLabel({
// owner: owner,
// repo: repo,
// issue_number: issue_number,
// name: label,
// });
}
core.info(
`Summarize checks against ${owner}/${repo}#${issue_number} will be: \nRemoving labels [${Array.from(labelContext.toRemove).join(", ")}] then \nAdding labels [${Array.from(labelContext.toAdd).join(", ")}]`,
);

if (labelContext.toAdd.size > 0) {
core.info(
`Adding labels: ${Array.from(labelContext.toAdd).join(", ")} to ${owner}/${repo}#${issue_number}.`,
);
// await github.rest.issues.addLabels({
// owner: owner,
// repo: repo,
// issue_number: issue_number,
// labels: Array.from(labelsToAdd),
// });
}
// for (const label of labelContext.toRemove) {
// core.info(`Removing label: ${label} from ${owner}/${repo}#${issue_number}.`);
// await github.rest.issues.removeLabel({
// owner: owner,
// repo: repo,
// issue_number: issue_number,
// name: label,
// });
// }

// if (labelContext.toAdd.size > 0) {
// core.info(
// `Adding labels: ${Array.from(labelContext.toAdd).join(", ")} to ${owner}/${repo}#${issue_number}.`,
// );
// await github.rest.issues.addLabels({
// owner: owner,
// repo: repo,
// issue_number: issue_number,
// labels: Array.from(labelContext.toAdd),
// });
// }

// adjust labelNames based on labelsToAdd/labelsToRemove
labelNames = labelNames.filter((name) => !labelContext.toRemove.has(name));
Expand All @@ -379,7 +379,7 @@
}
}

const commentBody = await createNextStepsComment(
const [commentBody, automatedChecksMet] = await createNextStepsComment(
core,
repo,
labelNames,
Expand All @@ -400,6 +400,10 @@
// commentName,
// commentBody
// )

core.info(
`Summarize checks has identified that status of "Automated merging requirements met" check should be updated to: ${automatedChecksMet}.`,
);
}

/**
Expand Down Expand Up @@ -727,7 +731,7 @@
* @param {string} targetBranch
* @param {CheckRunData[]} requiredRuns
* @param {CheckRunData[]} fyiRuns
* @returns {Promise<string>}
* @returns {Promise<[string, string]>}
*/
export async function createNextStepsComment(
core,
Expand All @@ -741,6 +745,8 @@
const requiredCheckInfos = requiredRuns
.filter((run) => checkRunIsSuccessful(run) === false)
.map((run) => run.checkInfo);

// determine if required runs have any successful runs.
const requiredCheckInfosPresent = requiredRuns.some((run) => {
const status = run.status.toLowerCase();
return status !== "queued" && status !== "in_progress";
Expand All @@ -749,7 +755,7 @@
.filter((run) => checkRunIsSuccessful(run) === false)
.map((run) => run.checkInfo);

const commentBody = await buildNextStepsToMergeCommentBody(
const [commentBody, automatedChecksMet] = await buildNextStepsToMergeCommentBody(
core,
labels,
`${repo}/${targetBranch}`,
Expand All @@ -758,7 +764,7 @@
fyiCheckInfos,
);

return commentBody;
return [commentBody, automatedChecksMet];
}

/**
Expand All @@ -768,7 +774,7 @@
* @param {boolean} requiredCheckInfosPresent
* @param {CheckMetadata[]} failingReqChecksInfo
* @param {CheckMetadata[]} failingFyiChecksInfo
* @returns {Promise<string>}
* @returns {Promise<[string, string]>}
*/
async function buildNextStepsToMergeCommentBody(
core,
Expand All @@ -794,7 +800,7 @@
const requirementsMet = !anyBlockerPresent && requiredCheckInfosPresent;

// Compose the body based on the current state
const commentBody = getCommentBody(
const [commentBody, automatedChecksMet] = getCommentBody(
requirementsMet,
anyBlockerPresent,
anyFyiPresent,
Expand All @@ -803,7 +809,7 @@
violatedReqLabelsRules,
);

return commentTitle + commentBody;
return [commentTitle + commentBody, automatedChecksMet];
}

/**
Expand All @@ -814,7 +820,7 @@
* @param {CheckMetadata[]} failingReqChecksInfo - Failing required checks info
* @param {CheckMetadata[]} failingFyiChecksInfo - Failing FYI checks info
* @param {RequiredLabelRule[]} violatedRequiredLabelsRules - Violated required label rules
* @returns {string} The body content HTML
* @returns {[string, string]} The body content HTML and the status that automated checks met should be set to.
*/
function getCommentBody(
requirementsMet,
Expand All @@ -825,10 +831,12 @@
violatedRequiredLabelsRules,
) {
let bodyProper = "";
let automatedChecksMet = "pending";

if (anyBlockerPresent || anyFyiPresent) {
if (anyBlockerPresent) {
bodyProper += getBlockerPresentBody(failingReqChecksInfo, violatedRequiredLabelsRules);
automatedChecksMet = "blocked";
}

if (anyBlockerPresent && anyFyiPresent) {
Expand All @@ -842,14 +850,15 @@
}
}
} else if (requirementsMet) {
automatedChecksMet = "success";
bodyProper =
`✅ All automated merging requirements have been met! ` +
`To get your PR merged, see <a href="https://aka.ms/azsdk/specreview/merge">aka.ms/azsdk/specreview/merge</a>.`;
} else {
bodyProper =
"⌛ Please wait. Next steps to merge this PR are being evaluated by automation. ⌛";
}
return bodyProper;
return [bodyProper, automatedChecksMet];
}

/**
Expand Down
7 changes: 1 addition & 6 deletions eng/tools/summarize-impact/src/PRContext.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { join, dirname } from "path";
import * as fs from "fs";

Expand All @@ -7,12 +7,7 @@
import { includesFolder } from "@azure-tools/specs-shared/path";
import { SpecModel } from "@azure-tools/specs-shared/spec-model";
import { Readme } from "@azure-tools/specs-shared/readme";
import {
swagger,
typespec,
example,
readme,
} from "@azure-tools/specs-shared/changed-files";
import { swagger, typespec, example, readme } from "@azure-tools/specs-shared/changed-files";

import { LabelContext } from "./labelling-types.js";
import { DiffResult, ReadmeTag, TagDiff, TagConfigDiff } from "./diff-types.js";
Expand Down
14 changes: 8 additions & 6 deletions eng/tools/summarize-impact/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/usr/bin/env node

import { evaluateImpact } from "./impact.js";
Expand Down Expand Up @@ -102,12 +102,14 @@
...(process.env.GITHUB_TOKEN && { auth: process.env.GITHUB_TOKEN }),
});

const labels = (await github.paginate(github.rest.issues.listLabelsOnIssue, {
owner,
repo,
issue_number: Number(prNumber),
per_page: 100,
})).map((label: any) => label.name);
const labels = (
await github.paginate(github.rest.issues.listLabelsOnIssue, {
owner,
repo,
issue_number: Number(prNumber),
per_page: 100,
})
).map((label: any) => label.name);

const labelContext: LabelContext = {
present: new Set(labels),
Expand Down
Loading