-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[many} Update all examples apps to use java 17 complile options #10195
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
[many} Update all examples apps to use java 17 complile options #10195
Conversation
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.
Code Review
This pull request updates the Java compile options to version 17 for a large number of example applications. The changes are generally correct, but I've identified a few inconsistencies. Two files set the jvmTarget
for Kotlin differently from the rest, and one file is missing the compileOptions
block altogether, which is the main purpose of this PR. My review comments provide suggestions to address these for consistency and completeness.
kotlinOptions { | ||
jvmTarget = '11' | ||
jvmTarget = JavaVersion.VERSION_17.toString() | ||
} |
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.
This file is missing the compileOptions
block to set the Java source and target compatibility to version 17. To complete the update for this example app, please add the following block, preferably before kotlinOptions
:
compileOptions {
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
}
b14dac0
to
e6d98c0
Compare
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.
LGTM
sourceCompatibility = JavaVersion.VERSION_11 | ||
targetCompatibility = JavaVersion.VERSION_11 | ||
sourceCompatibility = JavaVersion.VERSION_17 | ||
targetCompatibility = JavaVersion.VERSION_17 |
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.
nit: I think the README change technically requires a version bump, but it looks like this change won't affect clients anyways. So I'm fine with not needing to publish this change.
Part 1/2 for flutter/flutter/issues/176027
CHANGELOG exemption: Example apps only. #10186 (comment)
Tool test will come in part 2 after the non examples have been updated.
Pre-Review Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the [pub versioning philosophy], or I have commented below to indicate which [version change exemption] this PR falls under[^1].CHANGELOG.md
to add a description of the change, [following repository CHANGELOG style], or I have commented below to indicate which [CHANGELOG exemption] this PR falls under[^1].///
).