Copilot commented on code in PR #12814:
URL: https://github.com/apache/gluten/pull/12814#discussion_r3804869390
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -85,6 +106,16 @@ jobs:
- name: Detect changed paths
id: filter
run: |
+ if [ "${{ github.event_name }}" != "pull_request" ]; then
+ # schedule / workflow_dispatch: no PR diff to inspect -- run
+ # everything, including the extended matrices PR runs skip.
+ for flag in cpp java shims33 shims34 shims35 shims40 shims41
tools_it; do
+ echo "$flag=true" >> $GITHUB_OUTPUT
+ done
+ echo "full_run=true" >> $GITHUB_OUTPUT
+ exit 0
+ fi
Review Comment:
The comment says `full_run` is for schedule/workflow_dispatch runs, but the
condition enables `full_run` for *any* non-`pull_request` event (including
`push`). If this workflow runs on `push` (e.g., main/default branch), it will
unintentionally trigger the expanded matrices and chaos jobs on every
push/merge and largely negate the PR-time savings. Consider tightening the
condition to only match `schedule` + `workflow_dispatch` (or explicitly exclude
`push`), and keep `push` in the regular change-filtered path if applicable.
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -159,7 +200,16 @@ jobs:
matrix:
os: [ "ubuntu:22.04" ]
spark: [ "spark-3.3", "spark-3.4", "spark-3.5", "spark-4.0",
"spark-4.1" ]
- java: [ "java-8", "java-11", "java-17", "java-21", "java-25" ]
+ # PR runs test the primary JDK per Spark line (8 for 3.x, 17 for 4.x --
+ # after the static excludes below that is 6 combos instead of 11). The
+ # alternative-JDK combos (11/21/25) validate build + TPC run under JDKs
+ # that Gluten changes rarely break in a JDK-specific way; the nightly
+ # full_run keeps covering them daily so a break surfaces within a day
+ # instead of costing ~75 runner-minutes on every PR.
+ java: >-
+ ${{ fromJSON(needs.detect-changes.outputs.full_run == 'true'
+ && '["java-8", "java-11", "java-17", "java-21", "java-25"]'
+ || '["java-8", "java-17"]') }}
Review Comment:
The workflow repeats several long `fromJSON(...)` matrix expressions (Spark
versions, Java versions, Celeborn versions). This duplication makes it easier
for the nightly vs PR matrices to drift over time. Consider centralizing these
JSON lists (e.g., via reusable workflow inputs/outputs, job-level `env` JSON
strings, or YAML anchors) so changes to supported versions only need to be made
in one place.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
