Support listing multiple images to remove #1206
Conversation
| @@ -38,6 +38,12 @@ jobs: | |||
| run: ./gradlew functionalTest | |||
| - name: Documentation tests | |||
| run: ./gradlew docTest | |||
| - name: Upload test reports | |||
There was a problem hiding this comment.
There's really no need to upload test results, as you can view them with the help of a build scan. The build scan link is available at the end of each workflow run, on success and on failure. I'd say remove this change.
| @@ -23,5 +23,11 @@ jobs: | |||
| run: ./gradlew test | |||
| - name: Integration tests | |||
| run: ./gradlew integrationTest | |||
| - name: Upload test reports | |||
There was a problem hiding this comment.
There's really no need to upload test results, as you can view them with the help of a build scan. The build scan link is available at the end of each workflow run, on success and on failure. I'd say remove this change.
| private final Property<Boolean> force = getProject().getObjects().property(Boolean.class); | ||
| private final Property<Boolean> noPrune = getProject().getObjects().property(Boolean.class); | ||
|
|
||
| private final ListProperty<String> images = getProject().getObjects().listProperty(String.class); |
There was a problem hiding this comment.
Have a look at DockerPushImage. It does something similar.
| * @param images the names or IDs of the images. | ||
| * @see #images(ListProperty) | ||
| */ | ||
| public void images(List<String> images) { |
There was a problem hiding this comment.
You will just need a method that returns a reference to the images field.
@Input
public final SetProperty<String> getImages() {
return images;
}
| this.images.set(images); | ||
| } | ||
|
|
||
| // Overriding methods to provide a warning message. |
There was a problem hiding this comment.
Remove all of those methods and extend from AbstractDockerRemoteApiTask instead. The parent class DockerExistingImage is really only meant for a single image reference.
| @@ -42,6 +42,75 @@ class DockerRemoveImageFunctionalTest extends AbstractGroovyDslFunctionalTest { | |||
| !result.output.contains("repository") | |||
| } | |||
|
|
|||
| def "can remove multiple images"() { | |||
There was a problem hiding this comment.
We'll also want to change all documentation and test cases to use the new method instead.
| } | ||
|
|
||
| task removeImages(type: DockerRemoveImage) { | ||
| dependsOn buildFirstImage |
There was a problem hiding this comment.
We won't really need the dependsOn as you set an implicit dependency by assigning the image references (the output of DockerBuildImage tasks) to the input field images.
|
Thanks for the review, @bmuschko. I got busy and haven't had the time to finish this, but your comments will definitely help to get this done later. :-) |
|
I am going to close this issue. We can reopen if you or someone else is planning to continue working on this change. |
What?
Fixes #1190.
It allows users to list multiple images to remove.
How?
A new
ListPropertyallows users to set a list of the images they want to remove. The old property is maintained, but it will generate a warning if used and throw an error if used together with the list property.Status
This is a WIP/draft for now. But I think it is close to be ready to merge.