Yicong-Huang commented on code in PR #7703:
URL: https://github.com/apache/texera/pull/7703#discussion_r3896095574
##########
.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:
The aside lists edits and deletes, but the case that made this guard
necessary is this PR itself — it *adds* `sql/updates/README.md`, which is
neither. A `ddl-change` PR can also touch no `sql/updates/` file at all: the
labeler applies the label to any `**/*.sql`.
This comment is the only thing telling a future contributor why the pathspec
is `*.sql`. As written it omits the case that reason exists for. My miss from
last round.
```suggestion
# A ddl-change PR need not add a migration: it may edit or delete files
# under sql/updates/, add a non-.sql file such as this README, or touch
# a .sql file elsewhere in the repo. Any of those leaves operators
# nothing to apply, so no instruction goes out.
```
--
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]
