Fix Java 8 compatibiltiy #201
Conversation
Codecov Report
@@ Coverage Diff @@
## main #201 +/- ##
=========================================
Coverage 80.37% 80.37%
Complexity 1835 1835
=========================================
Files 100 100
Lines 7899 7899
Branches 1143 1143
=========================================
Hits 6349 6349
Misses 1033 1033
Partials 517 517
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Although the CEL source code was always compatible with Java 8, the compiled classes were not, because they were never build with `--release 8`. This change adds the changes to Java compilation. CI didn't catch this issue bug, because the while build was run with Java 8 and that caused the code to be _built_ again against Java 8. So this change also updates the CI workflow to run the tests against the classes built by Java 11.
snazy
force-pushed
the
fix-java-8-compatibility
branch
from
61efe7b to
c8b0d17
Compare
| @@ -32,17 +32,46 @@ jobs: | |||
| with: | |||
| submodules: 'true' | |||
|
|
|||
| # Special handling to test against Java 8: | |||
| # antlr version 4.10 requires Java 11 to run the antlr tool, but tests still need to run | |||
There was a problem hiding this comment.
nit: maybe extend to mention we compile with java11 as well but run with 8
| uses: gradle/gradle-build-action@v2 | ||
| with: | ||
| arguments: --rerun-tasks --info assemble check publishToMavenLocal -x jmh | ||
| arguments: --rerun-tasks assemble ${{ env.ADDITIONAL_GRADLE_OPTS }} check publishToMavenLocal -x jmh -x spotlessCheck |
There was a problem hiding this comment.
question: do the opts need to be inbetween task names? or could they come after --rerun-tasks instead?
There was a problem hiding this comment.
The order doesn't matter in this case.
| val javaToolchains = extensions.findByType(JavaToolchainService::class.java) | ||
| if (javaToolchains != null) { | ||
| val testWithJava8 = | ||
| tasks.register<Test>("testWithJava8") { |
There was a problem hiding this comment.
doesnt this mean that gradle execution in CI should mention a testWithJava8 task being executed?
i am not seeing it in the CI run on this PR 🤔
Although the CEL source code was always compatible with Java 8, the
compiled classes were not, because they were never build with
--release 8.This change adds the changes to Java compilation.
CI didn't catch this issue bug, because the while build was run
with Java 8 and that caused the code to be built again against
Java 8.
So this change also updates the CI workflow to run the tests against
the classes built by Java 11.