Skip to content

c8d/integration-cli: Adjust TestBuildClearCmd#46907

Merged
thaJeztah merged 1 commit intomoby:masterfrom
vvoland:c8d-fix-TestBuildClearCmd
Dec 7, 2023
Merged

c8d/integration-cli: Adjust TestBuildClearCmd#46907
thaJeztah merged 1 commit intomoby:masterfrom
vvoland:c8d-fix-TestBuildClearCmd

Conversation

@vvoland
Copy link
Copy Markdown
Contributor

@vvoland vvoland commented Dec 7, 2023

Config serialization performed by the graphdriver implementation maintained the distinction between an empty array and having no Cmd set.

With containerd integration we serialize the OCI types directly that use the omitempty option which doesn't persist that distinction.

Considering that both values should have exactly the same semantics (no cmd being passed) it should be fine if in this case the Cmd would be null instead of an empty array.

- What I did

- How I did it

- How to verify it
TestBuildClearCmd

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Config serialization performed by the graphdriver implementation
maintained the distinction between an empty array and having no Cmd set.

With containerd integration we serialize the OCI types directly that use
the `omitempty` option which doesn't persist that distinction.

Considering that both values should have exactly the same semantics (no
cmd being passed) it should be fine if in this case the Cmd would be
null instead of an empty array.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added status/2-code-review area/testing containerd-integration Issues and PRs related to containerd integration labels Dec 7, 2023
@vvoland vvoland added this to the 25.0.0 milestone Dec 7, 2023
@vvoland vvoland self-assigned this Dec 7, 2023
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing containerd-integration Issues and PRs related to containerd integration status/2-code-review

Projects

Development

Successfully merging this pull request may close these issues.

3 participants