-
Notifications
You must be signed in to change notification settings - Fork 664
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.
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
Some omitempty usage modifications #410
Conversation
@@ -17,34 +17,34 @@ package v1 | |||
// ImageConfig defines the execution parameters which should be used as a base when running a container using an image. | |||
type ImageConfig struct { | |||
// User defines the username or UID which the process in the container should run as. | |||
User string `json:"User"` | |||
User string `json:"User,omitepty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“omitepty” → “omitempty”.
Besides the typos, 5a318bf looks good to me. |
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
1 similar comment
Please revert this change. |
@stevvooe In addition, after reading your discussion, I am not very clear when |
Typically, |
@stevvooe So. I can't revert it,because the content have changed since it was merged. Would I need to reopen a PR to restore it? |
On Mon, Feb 06, 2017 at 07:43:01PM -0800, Stephen Day wrote:
Typically, `omitempty` should be used on every field unless you want
it to emit a zero-value. An example might be if you want to have an
empty slice be emitted, rather than omitting the field. I don't
think there are any cases in OCI where it make sense to not have
`omitempty`.
The manifest-list ‘manifests’ entry is currently explicitly REQUIRED
and it explicitly allows a zero-length array [1]. I'm not sure why we
don't just make it OPTIONAL (less work for the same amount of required
information), but as the spec stands it would not be a good idea to
make ManifestList.Manifests omitempty (as I pointed out in [2]).
[1]: https://github.com/opencontainers/image-spec/blame/v1.0.0-rc4/manifest-list.md#L23-L24
[2]: #557 (comment)
|
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Optional fields should be added omitempty
Signed-off-by: zhouhao zhouhao@cn.fujitsu.com