Copilot commented on code in PR #10156:
URL: https://github.com/apache/gravitino/pull/10156#discussion_r2878240831
##########
.github/workflows/backend-integration-test.yml:
##########
@@ -45,9 +48,39 @@ jobs:
- build.gradle.kts
- gradle.properties
- gradlew
- - setting.gradle.kts
+ - settings.gradle.kts
+ - name: Detect changed catalogs-contrib modules
+ id: contrib
+ run: |
+ if [ "${{ github.event_name }}" = "pull_request" ]; then
+ BASE_SHA="${{ github.event.pull_request.base.sha }}"
+ HEAD_SHA="${{ github.event.pull_request.head.sha }}"
+ else
+ BASE_SHA="${{ github.event.before }}"
+ HEAD_SHA="${{ github.sha }}"
+ fi
+
+ if [ -z "$BASE_SHA" ] || [ "$BASE_SHA" =
"0000000000000000000000000000000000000000" ]; then
+ BASE_SHA="$(git rev-list --max-parents=0 "$HEAD_SHA" | tail -n 1)"
+ fi
+
+ MERGE_BASE_SHA="$(git merge-base "$BASE_SHA" "$HEAD_SHA")"
Review Comment:
This step is running under GitHub Actions' default `bash -e -o pipefail`. If
`git merge-base` returns a non-zero exit code (e.g., no merge base), the script
will exit before your fallback `if [ -z "$MERGE_BASE_SHA" ]` runs. To make the
fallback effective, guard the command (e.g., `git merge-base ... || true`) and
then apply the empty-check.
```suggestion
MERGE_BASE_SHA="$(git merge-base "$BASE_SHA" "$HEAD_SHA" || true)"
```
##########
.github/workflows/frontend-integration-test.yml:
##########
@@ -24,24 +24,32 @@ jobs:
- .github/**
- api/**
- bin/**
+ - authorizations/**
+ - bundler/**
- catalogs/**
- - clients/client-java/**
+ - catalogs-contrib/**
+ - clients/**
- common/**
- conf/**
- core/**
- dev/**
- gradle/**
- iceberg/**
- - integration-test-common/**
- - meta/**
+ - lance/**
+ - maintenance/**
- scripts/**
- - server/**
+ - server/**
+ - meta/**
- server-common/**
Review Comment:
The `paths-filter` list no longer includes `integration-test-common/**`, but
`web-v2` integration tests depend on `:integration-test-common`. Changes in
`integration-test-common/` (e.g., test infra updates) will not trigger this
workflow, which can cause CI to miss relevant frontend test runs. Consider
adding `integration-test-common/**` back to the filter (or adding a broader
pattern that covers it).
```suggestion
- server-common/**
- integration-test-common/**
```
##########
.github/workflows/frontend-integration-test.yml:
##########
@@ -24,24 +24,32 @@ jobs:
- .github/**
- api/**
- bin/**
+ - authorizations/**
+ - bundler/**
Review Comment:
`bundler/**` is not a directory in this repo (there is `bundles/` instead).
With the current `paths-filter` config, changes under `bundles/` will not
trigger the frontend integration test workflow, and `bundler/**` will never
match anything. Update the pattern to the correct directory name (or remove it
if not needed).
```suggestion
- bundles/**
```
--
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]
