feat(gradle): KubernetesExtension has helper method to add image with builder #1888
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eclipse JKube CI ReportStarted new GH workflow run for #1888 (2022-11-07T09:05:49Z) ⚙️ JKube E2E Tests (3394166353)
|
Codecov Report
@@ Coverage Diff @@
## master #1888 +/- ##
=========================================
Coverage 53.05% 53.06%
- Complexity 3943 3944 +1
=========================================
Files 464 464
Lines 20750 20756 +6
Branches 2808 2808
=========================================
+ Hits 11009 11014 +5
Misses 8613 8613
- Partials 1128 1129 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
manusa
force-pushed
the
feat/kubernetes-extension-image-helper
branch
from
6d26050
to
796ec18
Compare
|
Thanks @manusa ! It will be of great interest for both Kotlin DSL users and people integrating your plugin into theirs (in plain Java).
I don't think you should. It's not Gradle idiomatic. Note that the current way of configuring is not Gradle idiomatic either. In Gradle, you should have properties which are mutable for the whole configuration phase. This is done so that plugins can provide conventional values, which can be overwritten by user configuration in build scripts, or other plugins (written in Java, Groovy, ...). Currently, your plugin is using the structures from the toolkit API which are immutable. This means that you assume that one the image is created (or any other nested element), its configuration is done and cannot be changed. This is not something that you should assume with Gradle. This makes this plugin particularly difficult to work with. By using a builder like in the Eventually, I would recommend that you have 2 separate data structures: one mutable, for configuration time, and one immutable, that you use at execution time. I reckon that this may be significant rework. I actually tried to give it a shot, but figured out that it would be difficult because of the use of Lombok. In any case, I appreciate that you're considering adding such a method, which will at least unblock our experiments. |
Thanks for your detailed explanation 🙌
When we started the Gradle plugins implementation, we didn't want to pollute the jkube-kit modules with Gradle-specific annotations and types. That's why we're mostly using the closure-deserialization hack to initialize the objects. Another thing that I'm not sure if you've tried, but could maybe be a middle-ground is using the Anyway, this PR should be released in 1.10.0 which should be out by next week 🤞 |
… builder Signed-off-by: Marc Nuri <marc@marcnuri.com>
manusa
force-pushed
the
feat/kubernetes-extension-image-helper
branch
from
796ec18
to
b51cb2f
Compare
|
Kudos, SonarCloud Quality Gate passed! |
rohanKanojia
approved these changes
Nov 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a convenience method to the
KubernetesExtensionto allow adding images using a builder:kubernetes { addImage {builder -> builder.name = 'the name' } }I was wondering if we would prefer something like the following snippet instead:
kubernetes { images { imageBuilder() .name('the-name') .build(imageBuildBuilder()./*...*/build()) } }/cc @melix
Type of change
test, version modification, documentation, etc.)
Checklist