ferenc-csaky commented on code in PR #48:
URL:
https://github.com/apache/flink-connector-shared-utils/pull/48#discussion_r2661776679
##########
pom.xml:
##########
@@ -60,9 +60,8 @@ under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
- <target.java.version>1.8</target.java.version>
- <maven.compiler.source>${target.java.version}</maven.compiler.source>
- <maven.compiler.target>${target.java.version}</maven.compiler.target>
+ <maven.compiler.source>17</maven.compiler.source>
+ <maven.compiler.target>17</maven.compiler.target>
Review Comment:
Since Flink 2.x still supports JDK11 IMO it is wrong to differ from that
here. If a connector drops JDK11 support specifically for whatever version it's
still fine, anybody who develops those features locally can use the
jdk17-target profile.
Mixing source/target versions also do not really makes too much sense in our
case (and I mean this for both the core Flink repo and this), as we have
separate profiles, and CI also runs with specific JDK versions.
So the only case these props actually matter is to a dev who loads up Flink
or connector code in their IDE, and what that recognizes by default. And if
that recognizes JDK17, and then only the build fails because it will acutally
run for JDK11, it's worse than opting to a newer JDK profile. At least that's
my take on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
