aglinxinyuan commented on PR #7703:
URL: https://github.com/apache/texera/pull/7703#issuecomment-5479003849
Both advisories addressed — one in code, one by correcting the description.
**Multi-line `$GITHUB_OUTPUT`.** Took your suggestion verbatim, `3cc6743`.
You're right that it's pre-existing and that this PR makes it less likely
rather than more; the `benchmarks-pr-comment.yml` precedent is what settled it
as worth the line.
**Proof run.** You're right that the description was wrong — it claimed the
change "cannot be exercised without merging a `ddl-change` PR", and a labeled
PR merged inside a fork does exercise the whole job. I've replaced that
sentence, but with a step-level proof rather than a fork run, because for this
particular finding the step-level version shows strictly more.
`Get added file in sql/updates/` is pure shell, so I ran its body verbatim
against real commit pairs on `main`, writing to a real `$GITHUB_OUTPUT` file,
then checked each file against the runner's rule that every line must be
`key=value`:
| PR shape | `base…head` | before | after |
| --- | --- | --- | --- |
| adds one migration | `516087ed^`…`516087ed` |
`sql_file=sql/updates/42.sql` — accepted | identical |
| adds two migrations | `7fbf64e3^`…`516087ed` |
`sql_file=sql/updates/41.sql` plus a bare `sql/updates/42.sql` line → `Invalid
format`, step fails, no notification | `sql_file=sql/updates/41.sql,
sql/updates/42.sql` — accepted, mail names both |
| this PR | `origin/main`…`HEAD` | `sql_file=` | `sql_file=` — `Send email`
skipped |
Why I'd rather have this than a green fork run: row 2's *before* column is
the finding. A fork run against the fixed workflow shows the fix passing — it
can't show what it fixed, since the pre-fix failure only exists in a revision
I'd have to construct and merge separately. And row 1 is the regression check
that single-migration PRs, the common case, come out byte-identical; a proof
run on this PR's own shape (row 3, nothing added) never reaches that path.
Two things a fork run would add that this doesn't, for the record: that
GHA's `if:` genuinely skips the step, and that `${FILE//$'\n'/, }` behaves the
same under the runner's `bash -e` as under my local bash. I'm treating both as
settled — there's no `shell:` override in the file, so it's default bash — but
say the word and I'll run it. My fork has no Actions secrets, so the `curl`
can't authenticate and nothing can reach dev@ either way.
`sql/updates/README.md` and the deletion itself are unchanged since round 02.
--
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]
