Make the rest of the modules forward compatible with Java 17 #199
Conversation
| @@ -68,8 +68,13 @@ dependencies { | |||
| implementation 'com.fasterxml.woodstox:woodstox-core:6.2.7' | |||
|
|
|||
| // open telemetry related classed. Latest Okhttp version is 4.10.0, pinning to 4.9.3 to avoid dependency issues | |||
There was a problem hiding this comment.
shall we move this comment to where ok_http3_version is defined
| @@ -7,7 +7,7 @@ ext { | |||
| jakarta_annotation_version = "1.3.5" | |||
| reactor_version = "3.4.3" | |||
| jodatime_version = "2.9.9" | |||
| junit_version = "4.13.1" | |||
| junit_version = "5.11.0" | |||
There was a problem hiding this comment.
shall we move this to top of build.gradle ?
There was a problem hiding this comment.
good catch, it's in the parent file already, will remove this duplicate
| @@ -71,6 +71,7 @@ shadowJar { | |||
| // e.g. java.lang.NoClassDefFoundError: openhouse/relocated/com/google/protobuf/GeneratedMessageV3 | |||
| // when users use fixtures library in their tests | |||
| exclude 'com.google.protobuf.**' | |||
| exclude 'com.google.gson.**' | |||
There was a problem hiding this comment.
it was the latest change i had to make, the reason is relocation rules didn't result in the same behavior between Java versions and I couldn't make imports work on both, but there is an option to just drop Java 8 compile support so we don't bother to with it, potentially if this is a problem we can drop Java 8
| implementation 'com.squareup.okio:okio:3.2.0' | ||
| implementation 'com.squareup.okio:okio-jvm:3.2.0' | ||
| implementation 'org.jetbrains.kotlin:kotlin-stdlib:2.0.20' |
There was a problem hiding this comment.
Is there a specific dependency on kotlin lib?
There was a problem hiding this comment.
it's pulled com.square.up, I'm pinning it here to a specific version
| implementation 'com.squareup.okio:okio:3.2.0' | ||
| implementation 'com.squareup.okio:okio-jvm:3.2.0' | ||
| implementation 'org.jetbrains.kotlin:kotlin-stdlib:2.0.20' | ||
| implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:2.0.20' |
There was a problem hiding this comment.
have we checked newly added libraries are ELR friendly?
There was a problem hiding this comment.
these dependencies are pulled transitively, i'm pinning the version
| @@ -142,3 +147,12 @@ shadowJar { | |||
| // By default shadow doesn't configure the build task to depend on the shadowJar task. | |||
| tasks.build.dependsOn tasks.shadowJar | |||
|
|
|||
| test { | |||
| if (JavaVersion.current() >= JavaVersion.VERSION_1_9) { | |||
There was a problem hiding this comment.
For my understanding, what are the usage of these args?
There was a problem hiding this comment.
at build time, if the java version is 9 or later we add these flag, these flags were introduced in java 9 when reflection API to change visibility of methods was removed
Can we update the testing done section docker testing for java 17 upgrade? |
Summary
The following changes were made:
The changes are expected to be non-consequential since the target bytecode is Java 8.
Changes
For all the boxes checked, please include additional details of the changes made in this pull request.
Testing Done
For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.
Additional Information
For all the boxes checked, include additional details of the changes made in this pull request.