remote: fix manifest add --annotation #15988
Conversation
github-actions
bot
added
the
kind/api-change
* `manifest add --annotation option` adds annotations field on remote environment. * `manifest inspect` prints annotations field on remote environment. Fixes: containers#15952 Signed-off-by: Toshiki Sonoda <[email protected]>
sstosh
force-pushed
the
manifest-annotate-remote
branch
from
7250433 to
32f54a8
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, sstosh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
| @@ -71,7 +71,7 @@ func Exists(ctx context.Context, name string, options *ExistsOptions) (bool, err | |||
| } | |||
|
|
|||
| // Inspect returns a manifest list for a given name. | |||
| func Inspect(ctx context.Context, name string, _ *InspectOptions) (*manifest.Schema2List, error) { | |||
| func Inspect(ctx context.Context, name string, _ *InspectOptions) (*libimage.ManifestListData, error) { | |||
There was a problem hiding this comment.
Sorry but we have to revert this, we cannot allow any breaking changes in pkg/bindings since this is supported for external uses as well
There was a problem hiding this comment.
Apologies for not catching that during the review, @sstosh.
What we probably need to do is to create a new function (and new option type). func InspectListData for instance.
| @@ -22,7 +22,7 @@ type ExistsOptions struct { | |||
| // AddOptions are optional options for adding manifest lists | |||
| type AddOptions struct { | |||
| All *bool | |||
| Annotation map[string]string | |||
| Annotation []string | |||
There was a problem hiding this comment.
We could keep the map and translate it to a slice if needed.
| Images []string // Images is an optional list of images to add/remove to/from manifest list depending on operation | ||
| OS *string // OS overrides the operating system for the image | ||
| All *bool // All when true, operate on all images in a manifest list that may be included in Images | ||
| Annotations []string // Annotations to add to manifest list |
|
#15992 to revert it for now, if you want to fix again please make sure to not change any public API in pkg/bindings. |
|
Thank you for your review. |
github-actions
bot
added
the
locked - please file new issue/PR
manifest add --annotation optionadds annotations field on remote environment.manifest inspectprints annotations field on remote environment.Fixes: #15952
Signed-off-by: Toshiki Sonoda [email protected]
Does this PR introduce a user-facing change?