Use mediatype helpers in more places #9155
Conversation
|
Hi @neersighted. 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. Instructions 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. |
| @@ -90,23 +90,21 @@ func MakeRefKey(ctx context.Context, desc ocispec.Descriptor) string { | |||
| // discovered in a call to Dispatch. Use with ChildrenHandler to do a full | |||
| // recursive fetch. | |||
| func FetchHandler(ingester content.Ingester, fetcher Fetcher) images.HandlerFunc { | |||
| return func(ctx context.Context, desc ocispec.Descriptor) (subdescs []ocispec.Descriptor, err error) { | |||
| return func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { | |||
There was a problem hiding this comment.
It's unclear to me why named returns were used here; after dropping the case scope this becomes an error (due to err previously being shadowed).
I've elected to drop this as everything becomes more clear with explicit returns in this otherwise trivial function.
| @@ -255,24 +255,22 @@ func (ts *localTransferService) pull(ctx context.Context, ir transfer.ImageFetch | |||
| } | |||
|
|
|||
| func fetchHandler(ingester content.Ingester, fetcher remotes.Fetcher, pt *ProgressTracker) images.HandlerFunc { | |||
| return func(ctx context.Context, desc ocispec.Descriptor) (subdescs []ocispec.Descriptor, err error) { | |||
| return func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { | |||
There was a problem hiding this comment.
This function appears to have been copied more-or-less wholesale from FetchHandler in handlers; my thoughts at https://github.com/containerd/containerd/pull/9155/files#r1339001829 apply equally here.
| @@ -71,17 +71,17 @@ func MakeRefKey(ctx context.Context, desc ocispec.Descriptor) string { | |||
| } | |||
| } | |||
|
|
|||
| switch mt := desc.MediaType; { | |||
| case mt == images.MediaTypeDockerSchema2Manifest || mt == ocispec.MediaTypeImageManifest: | |||
| switch { | |||
There was a problem hiding this comment.
I dropped the mt alias here as it really only was an advantage when doing the inline string equality checks; now that we are using the helpers, mt adds more line noise/makes this slower to read, in my opinion.
7cb3c7e to
68761c8
Compare
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
68761c8 to
5518a50
Compare
…h upstream containerd/main update fork-external/main branch to upstream containerd/main at commit f90f80d Related work items: containerd#8736, containerd#8861, containerd#8924, containerd#8934, containerd#9027, containerd#9076, containerd#9104, containerd#9118, containerd#9141, containerd#9155, containerd#9177, containerd#9183, containerd#9184, containerd#9186, containerd#9187, containerd#9191, containerd#9200, containerd#9205, containerd#9211, containerd#9214, containerd#9215, containerd#9221, containerd#9223, containerd#9228, containerd#9231, containerd#9234, containerd#9242, containerd#9246, containerd#9247, containerd#9251, containerd#9253, containerd#9254, containerd#9255, containerd#9268
Take advantage of the mediatype helpers from the
imagespackage throughout the rest of the project, also taking the opportunity to simplify some conditional logic where possible.