-
Notifications
You must be signed in to change notification settings - Fork 40.7k
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
Update Kotlin DSL examples that configure the environment of bootBuildImage to be additive #41173
Update Kotlin DSL examples that configure the environment of bootBuildImage to be additive #41173
Conversation
@@ -7,7 +7,7 @@ plugins { | |||
|
|||
// tag::env[] | |||
tasks.named<BootBuildImage>("bootBuildImage") { | |||
environment.set(environment.get() + mapOf("BP_JVM_VERSION" to "17")) | |||
environment.set(mapOf("BP_JVM_VERSION" to "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.
The intent is to add a BP_JVM_VERSION
to any existing entries. Won't this replace any existing entries in the environment map such that it now only contains BP_JVM_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.
environment
is a MapProperty
and it has a put(String, String)
method. I think that's what we should call. I'm not sure if there's some Kotlin DSL support for it, or if it should just be environment.put("BP_JVM_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.
Ah good point, I think I was confused by the fact the other samples in this page are also using environment.set(mapOf("key" to "value"))
. I guess we should fix those as well.
I think environment.put("BP_JVM_VERSION", "17")
is indeed what should be used. I have tested it successfully here. I don't think there is a better DSL than that.
Let me try to update other code samples accordingly.
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.
PR updated accordingly.
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.
Thanks.
1b43b3b
to
a7829e3
Compare
boot-build-image-env.gradle.kts
Thanks Sébastien! |
Reading Gradle documentation and seeing other similar code samples, I don't think the
environment.get()
part is needed, and it looks not idiomatic.