content: Add InfoReaderProvider#9607
Conversation
The interface that combines both content.InfoProvider and content.Provider was duplicated in multiple places - create one directly in `content` package and use it instead. Signed-off-by: Paweł Gronowski <[email protected]>
|
Hi @vvoland. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
removing these LGTM (nice find), but left a comment about deprecating -> aliasing, which we could consider including in a 1.x release to help transition.
After it's in the 1.x releases, it's fine to remove the aliases in main (v2.x).
|
|
||
| "github.com/opencontainers/go-digest" | ||
| imagedigest "github.com/opencontainers/go-digest" |
There was a problem hiding this comment.
Ugh; I still need to figure out which linter can catch these.
| // InfoProvider provides both content and info about content | ||
| type InfoProvider interface { |
There was a problem hiding this comment.
Do we know if there's external consumers of these interfaces? If so, we should add an alias and deprecate; more important if we decide to backport this patch (to help transition v1 -> v2).
// InfoProvider provides both content and info about content.
//
// Deprecated: use [content.InfoProvider] instead.
type InfoProvider = content.InfoProvider(same for the other ones)
There was a problem hiding this comment.
pkg/display was introduced in v2 and doesn't exist in v1 so we can skip the deprecation.
As for the pkg/cri/store/image technically it could have external consumers, but looking at https://pkg.go.dev/github.com/containerd/[email protected]/pkg/cri/store/image?tab=importedby and https://grep.app/search?q=pkg/cri/store/image it doesn't seem like it has any real publicly known importers.
Also, it's still exactly the same interface, so I think even if there are any consumers, the change should be transparent?
|
/ok-to-test |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
5 participants
The interface that combines both content.InfoProvider and content.Provider was duplicated in multiple places - create one directly in
contentpackage and use it instead.