mengw15 commented on code in PR #8096:
URL: https://github.com/apache/texera/pull/8096#discussion_r3885926115
##########
.github/workflows/backport-auto-label.yml:
##########
@@ -205,6 +210,13 @@ jobs:
for (const entry of entries) {
const label = entry.branch;
const manager = entry.manager;
+ // Say who has to sign off, so the label is not mistaken for
the
+ // decision. A manager who wrote the fix needs no approval —
+ // GitHub does not let anyone approve their own PR.
+ const gate =
+ manager && manager !== author
Review Comment:
Fixed in `f7dbbe3dc` — both comparisons, and the suppressed one on line 287,
now fold the logins before comparing, matching the gate in direct-backport-push.
##########
.github/workflows/direct-backport-push.yml:
##########
@@ -333,10 +440,81 @@ jobs:
core.info(`Push entries: ${JSON.stringify(pushEntries)}`);
core.info(`PR entries: ${JSON.stringify(prEntries)}`);
+ core.info(`Skipped entries: ${JSON.stringify(skippedEntries)}`);
core.setOutput("push_entries", JSON.stringify(pushEntries));
core.setOutput("pr_entries", JSON.stringify(prEntries));
core.setOutput("has_push", pushEntries.length > 0 ? "true" :
"false");
core.setOutput("has_pr", prEntries.length > 0 ? "true" : "false");
+ core.setOutput("skipped_entries", JSON.stringify(skippedEntries));
+ core.setOutput("has_skipped", skippedEntries.length > 0 ? "true" :
"false");
+
+ # A target held back for want of its release manager's approval leaves no
+ # other trace — no cherry-pick, no draft backport PR, no check — so say
so
+ # on the PR itself. One comment per PR, naming the manager whose approval
+ # would have carried the fix across, so a merge that outran the review is
+ # visible instead of silent.
+ - name: Report approval-gated targets
+ if: ${{ steps.discover.outputs.has_skipped == 'true' }}
+ uses: actions/github-script@v9
+ env:
+ SKIPPED: ${{ steps.discover.outputs.skipped_entries }}
+ with:
+ script: |
+ const skipped = JSON.parse(process.env.SKIPPED || "[]");
+ const { owner, repo } = context.repo;
+ const runUrl =
+
`${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`;
+
+ const byPr = new Map();
+ for (const entry of skipped) {
+ const list = byPr.get(entry.pr_number) || [];
+ list.push(entry);
+ byPr.set(entry.pr_number, list);
+ }
+
+ for (const [prNumber, entries] of byPr) {
+ // Why a target was held back decides what the manager does
+ // about it, so name the actual reason instead of a flat "not
+ // approved" — which reads as "you never looked at it" even when
+ // they approved and the approval was later dismissed.
+ const reason = (e) => {
+ if (e.state === "DISMISSED") {
+ return `@${e.manager} approved, but that approval was
dismissed before the merge`;
Review Comment:
Agreed — `DISMISSED` doesn't record what the review was, and dismissal isn't
restricted to approvals. Reworded to "…'s review was dismissed before the
merge, so no approval stands", so the comment states only what the state
establishes.
--
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]
