-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[BUILD] Tweak build params api for runtime version specific build logic #119212
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
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/es-delivery (Team:Delivery) |
@@ -32,7 +32,7 @@ tasks.matching { it.name == "compileMain21Java" }.configureEach { | |||
} | |||
|
|||
tasks.named('test').configure { | |||
if (buildParams.getRuntimeJavaVersion().map{ it.majorVersion.toInteger() }.get() >= 21) { | |||
buildParams.withMinimumJavaRuntimeVersion(JavaVersion.VERSION_21) { |
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.
I don't think we need this conditional on main anymore given the minimum runtime version is 21.
@@ -67,4 +67,6 @@ public interface BuildParameterExtension { | |||
Random getRandom(); | |||
|
|||
Boolean isGraalVmRuntime(); | |||
|
|||
void withMinimumJavaRuntimeVersion(JavaVersion version, Runnable runnable); |
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.
Forgive my bikeshedding: we use the term "minimum runtime version" to mean something slightly different. I also wonder if the name here should give some clue as to the fact that the given Runnable
is conditional. Some kind of "if" or "when" prefix might better. Perhaps ifJavaRuntimeVersion
or ifJavaRuntimeVersionCompatibleWith
?
No description provided.