-
Notifications
You must be signed in to change notification settings - Fork 863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to java 8 #1665
Switch to java 8 #1665
Conversation
… that we're really compiling for java 8/android 24.
@@ -148,8 +146,8 @@ configure(opentelemetryProjects) { | |||
mavenLocal() | |||
} | |||
|
|||
sourceCompatibility = 1.7 | |||
targetCompatibility = 1.7 | |||
sourceCompatibility = 1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can only use the new flag if we're compiling with java9+, though, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried switching to the new flag and it built locally with java 14, but failed to build on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we'll want to switch to using Java 11 to build on CI and drop support for building on older to take advantage of the new features. Even 15 would be ok there are usually new compilation features (often javadoc improvements) and still works on older. We would need to change the CI to have a build and separate test jdk configured with Gradle to continue testing with Java 8.
For another issue of course :)
Codecov Report
@@ Coverage Diff @@
## master #1665 +/- ##
============================================
- Coverage 86.70% 86.70% -0.01%
Complexity 1402 1402
============================================
Files 164 164
Lines 5545 5542 -3
Branches 554 553 -1
============================================
- Hits 4808 4805 -3
Misses 541 541
Partials 196 196
Continue to review full report at Codecov.
|
signature "org.codehaus.mojo.signature:java17:1.0@signature" | ||
signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature" | ||
signature "org.codehaus.mojo.signature:java18:1.0@signature" | ||
signature "net.sf.androidscents.signature:android-api-level-24:7.0_r2@signature" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okhttp uses API 21 signature and once before we had discussed following what they do to get Java 8 support in a way that follows the most used Android library. Is it possible to stick with that?
https://github.com/square/okhttp/blob/master/build.gradle#L153
Though if we just avoid CompletableFuture
I think any signature there should be fine too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on this one. If CompletableFuture
is the only thing we lose between 21 and 24 then I'm cool with that.
@@ -148,8 +146,8 @@ configure(opentelemetryProjects) { | |||
mavenLocal() | |||
} | |||
|
|||
sourceCompatibility = 1.7 | |||
targetCompatibility = 1.7 | |||
sourceCompatibility = 1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we'll want to switch to using Java 11 to build on CI and drop support for building on older to take advantage of the new features. Even 15 would be ok there are usually new compilation features (often javadoc improvements) and still works on older. We would need to change the CI to have a build and separate test jdk configured with Gradle to continue testing with Java 8.
For another issue of course :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can revisit the Android API level later let's go ahead and merge first!
* switch to java 8 * update the READMEs and use Object.equals in a couple of cases to test that we're really compiling for java 8/android 24. * formatting * use the newer release options for the build, and change the int test to not be java 7 any more. * switch back to source/target compatibility * sure wish I could run docker locally to test this out.
And Android level 24