mengw15 commented on code in PR #8096:
URL: https://github.com/apache/texera/pull/8096#discussion_r3885967220
##########
.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:
Good catch — `get(target) || ""` collapsed "absent from the config" into
"listed with no manager", and it is reachable: retiring a branch means dropping
its entry entirely while its `release/*` label lives on, and targets are
derived from labels alone. Fixed in `cd5819bc1`: an unlisted target is now held
back and reported ("not listed in .github/release-branches.yml … remove the
label, or add the branch back"), and only a present entry with an empty manager
stays 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]
