Enforce compatibility with JDK 8 when compiling on later JDKs #1909
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bartekpacia
force-pushed
the
gradle/set_jdk_target
branch
from
f165eb5
to
e221b1b
Compare
|
Weird failures when building Weird failuresUpdate It's KT-52815. Bumping Kotlin 1.8.0 -> 1.8.22 solves this. |
bartekpacia
force-pushed
the
gradle/set_jdk_target
branch
from
f6b6cca
to
4e07355
Compare
bartekpacia
force-pushed
the
gradle/set_jdk_target
branch
from
a119d96
to
9a5e5a9
Compare
bartekpacia
force-pushed
the
gradle/set_jdk_target
branch
from
9a5e5a9
to
4006fc8
Compare
|
Removing If we do discover some regression in the future though, we can try using kfswatch instead of depending on hidden JDK APIs. |
igorsmotto
reviewed
Aug 13, 2024
.github/workflows/test-e2e.yaml
Outdated
| @@ -113,7 +113,7 @@ jobs: | |||
| uses: actions/setup-java@v4 | |||
| with: | |||
| distribution: zulu | |||
| java-version: 8 | |||
| java-version: 11 | |||
There was a problem hiding this comment.
- make it possible to build Maestro on all Java 11 and higher. Compilation is faster with each release, and has other advantages
- keep minimum Java required to run Maestro at 8. Note: for building, JDK11 will be the new minimum.
Following your comment, this test-e2e/test should work with java-version: 8 right? Otherwise, this means that maestro is unable to run at 8.
There was a problem hiding this comment.
this test-e2e/test should work with java-version: 8 right?
This is correct. (This is actually one of the things I wanted to accomplish with the test-e2e.yaml - to always verify we work on Java 8)
Now, this change (in test-cloud job) is actually a mistake, it should've been 8.
On Android (the test-local job) it's currently not 8, because none of the "setup Android & start emulator" GitHub Actions support installing an older SDK Command-Line Tools release (newer require either Java 11 or 17). I'm hoping this will change soon.
There was a problem hiding this comment.
So that means we can only test if maestro cloud works on java 8? Actually using maestro locally it's still TBD? Perhaps we could use devices from Maestro Cloud and connect them remotely using adb and therefore use the maestro cli?
There was a problem hiding this comment.
Actually using maestro locally it's still TBD?
It already works as well, but the workaround I used is to compile on Java 8, and then run on Java 11 (because Android tools require at least Java 11). But now we will not be able to compile anymore on Java 8, so we have to find a way run on Java 8.
Perhaps we could use devices from Maestro Cloud and connect them remotely using adb and therefore use the maestro cli?
We could, but this sounds a bit complex (but I'd love to discuss more if you think it makes sense here). For now though I'd prefer to go with the simplest possible solution - find a way to use old Android Command-Line Tools+Java 8 and run emulator on GitHub Actions.
This was referenced Aug 15, 2024
Closed
|
Constantly changing between JDK versions is really annoying. I gave this PR a good test (run In addition to that, I'm pretty confident that Going forward with the merge. |
This was referenced Aug 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
follow-up of #1880
aimed at #1801
With these changes I aim to:
Notes: