Copilot commented on code in PR #8096:
URL: https://github.com/apache/texera/pull/8096#discussion_r3885859456
##########
.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:
`DISMISSED` does not prove that the manager previously approved: GitHub also
allows a `CHANGES_REQUESTED` review to be dismissed, and both appear here as
`DISMISSED`. In that case this comment falsely tells readers that an approval
existed. Use wording that identifies a dismissed review without asserting its
original state.
##########
.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:
GitHub logins are case-insensitive, but this comparison is case-sensitive.
If the configured manager differs from the author's canonical casing, the
report incorrectly says the author must approve even though the post-merge gate
treats them as approved. Normalize both values before comparing.
This issue also appears on line 284 of the same file.
--
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]
