Conversation
|
|
||
| <properties> | ||
| <surefire.rerunFailingTestsCount>0</surefire.rerunFailingTestsCount> | ||
| <maven.compiler.release>17</maven.compiler.release> |
There was a problem hiding this comment.
why is this needed? seems worth a comment
There was a problem hiding this comment.
Maybe:
Using JDK 11 language level restriction until Druid 37 which (as of 2025-12-10) is the current target for removing Hadoop support from the dev list threads. Hadoop relies upon JDK 8/11 language level which breaks when upgraded to anything higher.
from: #18759 (comment)
There was a problem hiding this comment.
Thanks all, I added some explanation.
There was a problem hiding this comment.
we will also need to remember to remove the changes here and in static-checks.yml in the future, not sure how to best manage that debt, i guess we could put a note in the root pom.xml...
imo we should try to avoid diverging like this in any other modules and try to stay consistent with the root pom unless there is a seriously good reason so that we don't have to keep track of stuff like this.
There was a problem hiding this comment.
I think we may fix it up in this patch itself. The only requirement for 17 (AFAIK) is the doc-style strings added in a recent test NestedDataFormatsTest. For the time being, we can just stick to regular strings.
There was a problem hiding this comment.
update the root pom to include things to check for upgrade to 17.
imo using 17 in embedded-test is safe and multi-line string syntax helps with readability a lot, so hopefully we dont let this java 11 thing drag up back.
|
@cecemei good to merge? |
Description
embedded-testmodule, force use java 17 to support the multi-line string syntax, and exclude it from strict compilation.This PR has: