klion26 commented on code in PR #3568:
URL: https://github.com/apache/amoro/pull/3568#discussion_r2106782058
##########
.github/workflows/core-hadoop3-ci.yml:
##########
@@ -35,21 +35,25 @@ on:
jobs:
build:
runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ jdk: [ { setup: '8', maven: '1.8' }, { setup: '11', maven: '11' } ]
Review Comment:
seems as above
##########
amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml:
##########
@@ -147,6 +147,10 @@
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
</exclusion>
+ <exclusion>
Review Comment:
not sure do we still need to support spark 3.2
##########
amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml:
##########
@@ -147,6 +147,10 @@
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
</exclusion>
+ <exclusion>
Review Comment:
why do we need to exclude this here
##########
.github/workflows/docker-images.yml:
##########
@@ -41,12 +41,13 @@ jobs:
strategy:
matrix:
hadoop: [ "v2", "v3" ]
+ jdk: [ "11" ]
Review Comment:
why do we need to hardcode the jdk version here
##########
.github/workflows/core-hadoop2-ci.yml:
##########
@@ -35,21 +35,25 @@ on:
jobs:
build:
runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ jdk: [ { setup: '8', maven: '1.8' }, { setup: '11', maven: '11' } ]
Review Comment:
seems the value(`1.8`) is the version of jdk, why do we name it with `maven`
here?
##########
.github/workflows/docker-images.yml:
##########
@@ -237,7 +240,7 @@ jobs:
&& echo "AMORO_VERSION=${AMORO_VERSION}" >> $GITHUB_OUTPUT
- name: Build optimizer module with Maven
- run: mvn clean package -pl 'amoro-optimizer/amoro-optimizer-spark' -am
-e ${OPTIMIZER_SPARK} -DskipTests -B -ntp
+ run: mvn clean package -pl 'amoro-optimizer/amoro-optimizer-spark' -am
-e ${OPTIMIZER_SPARK} -DskipTests -B -ntp -Djava.version=${{ matrix.jdk }}
Review Comment:
Do we need to set `maven.compiler.source` and `maven.compiler.target` in the
command line?
executed on my local machine with `mvn clean package -Djava.version=11`, but
it did not run with jdk11, maybe we need to set the properties
`maven.compiler.{source/target}`?
--
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]