Remove Gradle error from newer JDK versions #2392
Conversation
pablisco
changed the title
|
Thanks @pablisco !! It should be checked with newer JDK versions because it's being checked with JDK 8 here. I'll review it asap! |
| "Building with a JRE or JDK9 is currently not supported.") | ||
| testCompileOnly files(toolsJar) | ||
| if (toolsJar) { | ||
| testCompileOnly files(toolsJar) |
There was a problem hiding this comment.
Meanwhile you could check if it's possible to remove this dependency for testing to have the same behavior with JDK8 or higher.
There was a problem hiding this comment.
Just tested clean build with JDK 8 and it doesn't seem to need it :) Removing it: a353f73
There was a problem hiding this comment.
Great, thanks @pablisco !! 👏 I'd miss 2 tasks:
- Just JDK8 is checked here so I miss adding checks for other JDKs.
- Update the documentation.
I'll help you asap!!
There was a problem hiding this comment.
@rachelcarmena I've updated the documentation (with the JDK 16 caveat) and updated the build tests to test on multiple JDK versions
.github/workflows/build.yml
Outdated
| os: [ubuntu-latest, macos-latest] | ||
| java: [8, 8.0.192, 11, 11.0.2, 13, 13.0.4, 15, 16-ea] | ||
| os: [macos-latest] | ||
| java: [8, 11, 13, 15, 16-ea] |
There was a problem hiding this comment.
Java 16 is GA now IIRC, so I think this should be updated to remove the -ea part. Also, actions/setup-java@v2 is available now.
There was a problem hiding this comment.
Good shout 🙇 Updating.
| @@ -430,6 +430,7 @@ interface TypeElementEncoder : KotlinMetatadataEncoder, KotlinPoetEncoder, Proce | |||
| Modifier.SYNCHRONIZED -> null | |||
| Modifier.NATIVE -> null | |||
| Modifier.STRICTFP -> null | |||
| else -> null | |||
There was a problem hiding this comment.
This could be handled move gracefully depending on the JDK version but it's out of the scope of this ticket. Happy to add a new one to look at ways to support newer modifiers.
cc: @raulraja
There was a problem hiding this comment.
This is regarding SEALED and NON_SEALED modifiers introduced in JDK 15
There was a problem hiding this comment.
whatever works, most of this code in the old meta part is going away soon and the new meta does not depend on the TypeeElement java apis.
.github/workflows/build.yml
Outdated
| matrix: | ||
| os: [macos-latest] | ||
| java: [8, 11, 13, 15] | ||
| name: Build libraries with JDK ${{ matrix.java }} on ${{ matrix.os }} |
There was a problem hiding this comment.
Wondering if we want to build on all these versions. I was thinking about splitting github actions to build/test jvm and javascript in parallel. Since the build went up considerable now we added MPP.
Before 4-5 minutes for a build, now ~20 minuntes.
But for every MPP library we generate 4 JS binaries + JVM, and soon native which has a lot of targets as well.
How does this impact CI time? How many Github Actions can we run in parallel? 🤔
There was a problem hiding this comment.
I think they are run in Parallel as far as I can tell, so we should be safe 🤞
There was a problem hiding this comment.
Just checked, and it seems that way 🥳
|
@pablisco looking at the times every task takes we're running JS tests on all 4 of those actions. |
Closes #2391
This PR potentially removes the requirement to have JDK 8 installed in our machines 🤞