Skip to content
This repository was archived by the owner on Jan 15, 2026. It is now read-only.

cmd: Modify the assignment of parameters#137

Merged
coolljt0725 merged 1 commit into
opencontainers:masterfrom
zhouhao3:value-test
Apr 10, 2017
Merged

cmd: Modify the assignment of parameters#137
coolljt0725 merged 1 commit into
opencontainers:masterfrom
zhouhao3:value-test

Conversation

@zhouhao3
Copy link
Copy Markdown

@zhouhao3 zhouhao3 commented Apr 6, 2017

This modification can avoid the absence of default values.

Signed-off-by: zhouhao zhouhao@cn.fujitsu.com

@xiekeyang
Copy link
Copy Markdown
Contributor

xiekeyang commented Apr 6, 2017

I'd rather to set value at statement of v, like,

v := bundleCmd{root : context.rootfs, ref : context.ref }

And, I think if condition might be necessary for those who has no default value.

@vbatts
Copy link
Copy Markdown
Member

vbatts commented Apr 6, 2017

what happens if those variables are not set? will it panic?

@xiekeyang
Copy link
Copy Markdown
Contributor

@vbatts

what happens if those variables are not set? will it panic?

it doesn't cause panic. The value should be empty for string type, false for bool type, and so on.

Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@zhouhao3
Copy link
Copy Markdown
Author

And, I think if condition might be necessary for those who has no default value.

@xiekeyang The if statement performs only the assignment action, and the previous assignment has been repeated, so I do not think so.

@xiekeyang
Copy link
Copy Markdown
Contributor

xiekeyang commented Apr 10, 2017

LGTM

Approved with PullApprove

1 similar comment
@coolljt0725
Copy link
Copy Markdown
Member

coolljt0725 commented Apr 10, 2017

LGTM

Approved with PullApprove

@coolljt0725 coolljt0725 merged commit f47425d into opencontainers:master Apr 10, 2017
@zhouhao3 zhouhao3 deleted the value-test branch April 10, 2017 09:00
@xiekeyang xiekeyang mentioned this pull request Jun 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants