Skip to content

serialization: Entrypoint and Cmd nullable #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
vbatts merged 2 commits into from
Jun 21, 2016
Merged

serialization: Entrypoint and Cmd nullable #152

vbatts merged 2 commits into from
Jun 21, 2016

Conversation

@runcom
Copy link
Member

@runcom runcom commented Jun 16, 2016 •
edited
Loading

This is built on top of #151 - refer only to the last commit for review

@philips feel free to either cherry-pick my commit from here or wait #151 to be merged first (and I'll rebase)

This patch makes Entrypoint and Cmd fields in the serialization schema nullable + adds tests

/cc @vbatts

@runcom runcom mentioned this pull request Jun 17, 2016
Merged
@runcom runcom force-pushed the validation branch 2 times, most recently from 66d5a5c to 24280af Compare June 17, 2016 16:26
@vbatts
Copy link
Member

vbatts commented Jun 20, 2016

not sure about this. If it is nullable, then it can be omitted. Right?

@runcom
Copy link
Member Author

runcom commented Jun 20, 2016

it should be the same as Volumes - it's not omitted, you get it as Volumes: null in the config and digest changes w/ and w/o it (this is about Docker v2s2 backward compatibility)

@vbatts
Copy link
Member

vbatts commented Jun 20, 2016

i see. Could you rebase?

Brandon Philips and others added 2 commits June 21, 2016 01:56
@runcom
serialization: add test, make volumes null
e0afe63 
This is a spec change based on testing live Docker Hub configs. Volume
may be null as found in the hub.docker.com/library/docker image.

Signed-off-by: Brandon Philips <brandon.philips@coreos.com>
@runcom
serialization: Entrypoint and Cmd nullable
d365b79 
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom
Copy link
Member Author

runcom commented Jun 20, 2016

@vbatts done

@vbatts
Copy link
Member

vbatts commented Jun 21, 2016 •
edited by caniszczyk
Loading

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Jun 21, 2016

this is unfortunate :-(

@jonboulle
Copy link
Contributor

jonboulle commented Jun 21, 2016 •
edited by caniszczyk
Loading

LGTM I guess

Approved with PullApprove

@vbatts vbatts merged commit 7871606 into opencontainers:master Jun 21, 2016
@runcom runcom deleted the validation branch June 21, 2016 14:41
@runcom runcom mentioned this pull request Jun 21, 2016
Closed
@wking wking mentioned this pull request Sep 19, 2016
Closed
@wking wking mentioned this pull request Oct 6, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Ghostman2591 Ghostman2591 Ghostman2591 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@runcom @vbatts @jonboulle @Ghostman2591