Yicong-Huang commented on code in PR #7703:
URL: https://github.com/apache/texera/pull/7703#discussion_r3896930565
##########
.github/workflows/automatic-email-notif-on-ddl-change.yml:
##########
@@ -40,9 +40,16 @@ jobs:
FILE=$(git diff --name-only --diff-filter=A \
${{ github.event.pull_request.base.sha }} \
${{ github.event.pull_request.merge_commit_sha }} \
- -- 'sql/updates/')
- echo "sql_file=$FILE" >> $GITHUB_OUTPUT
+ -- 'sql/updates/*.sql')
+ # git diff prints one path per line, so a PR adding two migrations
+ # would put a bare second line in $GITHUB_OUTPUT -- no key=, which
+ # makes the runner reject the whole file and the step fail.
+ echo "sql_file=${FILE//$'\n'/, }" >> $GITHUB_OUTPUT
+ # A ddl-change PR that adds no migration -- it only edits or deletes
+ # files under sql/updates/ -- leaves operators nothing to apply, so it
+ # must not mail out an instruction naming an empty file.
Review Comment:
Verified: `.github/workflows/automatic-email-notif-on-ddl-change.yml:48-51`
now names all four shapes, applied verbatim in `16aaa2c`. Checked the fourth
clause against the config rather than the diff — `.github/labeler.yml:138-141`
applies `ddl-change` on `**/*.sql`, so a `.sql` anywhere in the repo does carry
the label with nothing added under `sql/updates/`. PyYAML still loads the file
and the `if:` still binds to `Send email`. Resolving.
##########
.github/workflows/automatic-email-notif-on-ddl-change.yml:
##########
@@ -40,9 +40,16 @@ jobs:
FILE=$(git diff --name-only --diff-filter=A \
${{ github.event.pull_request.base.sha }} \
${{ github.event.pull_request.merge_commit_sha }} \
- -- 'sql/updates/')
- echo "sql_file=$FILE" >> $GITHUB_OUTPUT
+ -- 'sql/updates/*.sql')
+ # git diff prints one path per line, so a PR adding two migrations
+ # would put a bare second line in $GITHUB_OUTPUT -- no key=, which
+ # makes the runner reject the whole file and the step fail.
+ echo "sql_file=${FILE//$'\n'/, }" >> $GITHUB_OUTPUT
+ # A ddl-change PR that adds no migration -- it only edits or deletes
+ # files under sql/updates/ -- leaves operators nothing to apply, so it
+ # must not mail out an instruction naming an empty file.
Review Comment:
Duplicate of the thread above — I posted the same finding twice last round.
Same fix, same verification. Resolving.
--
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]
