Keep duplicate layers to match history #1017
Conversation
There was a problem hiding this comment.
I feel like this will work, but punting to @coollog for final check.
| @@ -34,7 +34,7 @@ | |||
| static class Builder { | |||
|
|
|||
| private final ImageLayers.Builder<CachedLayerWithMetadata> layersBuilder = | |||
| ImageLayers.builder(); | |||
| ImageLayers.<CachedLayerWithMetadata>builder().removeDuplicates(); | |||
There was a problem hiding this comment.
Is <CachedLayerWithMetadata> necessary?
There was a problem hiding this comment.
Yeah, get compiler errors otherwise as it resolves to ImageLayers<Layer>.
| public ImageLayers<T> build() { | ||
| return new ImageLayers<>(ImmutableList.copyOf(layers), layerDigestsBuilder.build()); | ||
| ImmutableList<T> layers; |
There was a problem hiding this comment.
What if we remove this variable assignment and push return into each branch of if? Disregard my comment if it won't look any better though.
|
Also add a bug fix entry in |
There was a problem hiding this comment.
I'm thinking maybe ImageLayers.Builder should be instantiated with removeDuplicates as true or false via ImageLayers.builder(boolean removeDuplcates) so that it does add differently based on the value of removeDuplicates rather than having ImageLayers reconstruct a LinkedHashSet for the removeDuplicates=true case.
|
I think it makes more sense to have the logic in |
Fixes #991