Copilot commented on code in PR #8096:
URL: https://github.com/apache/texera/pull/8096#discussion_r3885934154
##########
.github/workflows/direct-backport-push.yml:
##########
@@ -270,15 +272,101 @@ jobs:
])
);
+ // Whether a fix reaches a release branch is that branch's release
+ // manager's call, so a `release/*` label only *nominates* a
target:
+ // the manager named in release-branches.yml must have approved the
+ // PR for anything to land on their branch.
+ //
+ // "Approved" is the manager's *latest* review state. COMMENTED
+ // reviews never change an approval, and a later CHANGES_REQUESTED
+ // or DISMISSED revokes an earlier one. Any dismissal lands on
+ // DISMISSED — a stale approval cleared by a push, or a blocking
+ // review dismissed by hand — so the state proves an approval is
+ // not standing, never what the dismissed review had been.
+ // listReviews returns reviews oldest-first (documented), so the
+ // last state recorded for a login is the one that counts.
+ async function managerReviewStates(pullRequest) {
+ const states = new Map();
+ try {
+ const reviews = await
github.paginate(github.rest.pulls.listReviews, {
+ owner,
+ repo,
+ pull_number: pullRequest.number,
+ per_page: 100,
+ });
+ for (const review of reviews) {
+ const login = review.user?.login;
+ if (!login || review.state === "COMMENTED") continue;
+ states.set(login.toLowerCase(), review.state);
+ }
+ } catch (e) {
+ // Without the review list we cannot prove an approval, and
+ // backporting unreviewed is the worse failure, so every gated
+ // target is held back. `null` (not an empty map) keeps that
+ // distinct from "read fine, nobody approved" — we must not
+ // report that a manager did not approve when we never looked.
+ core.warning(
+ `listReviews for #${pullRequest.number} failed ` +
+ `(${e.status ?? "?"}): ${e.message}. Treating every gated ` +
+ `target as unapproved.`
+ );
+ return null;
+ }
+ return states;
+ }
+
+ // Split the labeled targets into the ones their release manager
has
+ // signed off on and the ones still waiting. Managers gate their
own
+ // branch and nothing else, so the approvals compose: with v1.2 and
+ // v1.3 both labeled, an approval from the v1.2 manager alone sends
+ // the fix to v1.2 and leaves v1.3 behind.
+ function splitByApproval(pullRequest, targets, states) {
+ const author = (pullRequest.user?.login || "").toLowerCase();
+ const approved = [];
+ const unapproved = [];
+ for (const target of targets) {
+ const manager = releaseManagers.get(target) || "";
+ if (!manager) {
+ core.info(`${target}: no release manager configured;
ungated.`);
+ approved.push(target);
Review Comment:
An unconfigured `release/*` label is currently treated the same as a
configured target that intentionally omits `manager`, so it bypasses the new
approval gate and can be pushed automatically. This also means a label left
behind after an EOL branch is removed from `release-branches.yml` remains
active, despite that file being the source of truth. Check
`releaseManagers.has(target)` separately and reject/report unknown targets;
only a present entry with an empty manager should be ungated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
