Copilot commented on code in PR #13040:
URL: https://github.com/apache/gluten/pull/13040#discussion_r4027938291
##########
.github/workflows/velox_nightly.yml:
##########
@@ -486,8 +487,7 @@ jobs:
./build/mvn clean install -Pspark-3.4 -Pjava-17 -Pbackends-velox
-Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests
-Dmaven.source.skip
./build/mvn clean install -Pspark-3.5 -Pjava-17 -Pbackends-velox
-Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests
-Dmaven.source.skip
./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-17
-Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon
-DskipTests -Dmaven.source.skip
- # iceberg/hudi support for spark-4.1 with java-17 is not avaiable,
skip it first
- ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-17
-Pbackends-velox -Pceleborn -Puniffle -Pdelta -DskipTests -Dmaven.source.skip
+ ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-17
-Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon
-DskipTests -Dmaven.source.skip
Review Comment:
This job previously documented that Iceberg/Hudi support for Spark 4.1 with
Java 17 was not available and intentionally skipped those profiles. This change
now enables `-Piceberg` and `-Phudi` for Spark 4.1 on Java 17 without any
accompanying changes in this PR that would make that support available, which
risks re-breaking the nightly workflow. If Spark 4.1 + Java 17 still cannot
build Iceberg/Hudi, consider restoring the prior exclusion (omit `-Piceberg
-Phudi`) or splitting into separate commands/jobs (e.g., Spark 4.1 Java 17
without Iceberg/Hudi and Spark 4.1 Java 21 with Iceberg/Hudi).
##########
.github/workflows/velox_nightly.yml:
##########
@@ -137,6 +137,7 @@ jobs:
./build/mvn clean install -Pspark-3.4 -Pjava-17 -Pbackends-velox
-Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests
-Dmaven.source.skip
./build/mvn clean install -Pspark-3.5 -Pjava-17 -Pbackends-velox
-Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon -DskipTests
-Dmaven.source.skip
./build/mvn clean install -Pspark-4.0 -Pscala-2.13 -Pjava-17
-Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon
-DskipTests -Dmaven.source.skip
+ ./build/mvn clean install -Pspark-4.1 -Pscala-2.13 -Pjava-17
-Pbackends-velox -Pceleborn -Puniffle -Piceberg -Phudi -Pdelta -Ppaimon
-DskipTests -Dmaven.source.skip
Review Comment:
The workflow relies on multiple copy-pasted Maven invocations per Spark
version, which already led to inconsistencies (e.g., missing `-Ppaimon` in some
Spark 4.1 commands before this PR). To prevent future drift, consider
refactoring these into a matrix (Spark version × Java version × feature flags)
or a reusable composite action so options like `-Ppaimon` are defined once and
applied consistently.
--
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]
