Skip to content
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

Fix type of devices type #323

Merged
merged 2 commits into from
Feb 29, 2016
Merged

Conversation

hqhq
Copy link
Contributor

@hqhq hqhq commented Feb 17, 2016

Fixes: opencontainers/runc#566

For type rune, we can assign char as 'c' in struct, but after
marshal, it'll be presented as int32. So it should be a
number in our example of json file. Otherwise we'll got error:
json: cannot unmarshal string into Go value of type int32
If we trying to unmarshal the json file.

Signed-off-by: Qiang Huang h.huangqiang@huawei.com

@hqhq
Copy link
Contributor Author

hqhq commented Feb 17, 2016

I found the example of fileMode is also wrong, pushing a patch to fix it.

@crosbymichael crosbymichael added this to the v0.4.0 milestone Feb 22, 2016
@crosbymichael
Copy link
Member

I think we should change it from rune to string so that you can actually write "b", "c" in the json spec and you can easily know what type of device it is. What do you think?

@vishh
Copy link
Contributor

vishh commented Feb 22, 2016

+1 for switching to string.

@wking
Copy link
Contributor

wking commented Feb 22, 2016

On Mon, Feb 22, 2016 at 11:32:06AM -0800, Vish Kannan wrote:

+1 for switching to string.

ditto

@hqhq hqhq changed the title Fix json example of device Fix type of devices type Feb 23, 2016
@hqhq
Copy link
Contributor Author

hqhq commented Feb 23, 2016

Updated, thanks.

@wking
Copy link
Contributor

wking commented Feb 23, 2016

On Mon, Feb 22, 2016 at 05:24:03PM -0800, Qiang Huang wrote:

Updated, thanks.

I prefer backticks to quotes for Markdown literals, but other than
that minor quibble, both 9cd1ae5 and 96c5ff0 look good to me.

Fixes: opencontainers/runc#566

For type rune, we can assign char as 'c' in struct, but after
marshal, it'll be presented as int32. So in json config it needs
to be presented as a number which is not friendly to be identified.

Change it to string so that you can actually write "b", "c" in json
spec and you can easily know what type of device it is.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
In json, os.FileMode would be presented as a uint32, which
is decimal. Otherwise we'll get error:
`invalid character '6' after object key:value pair`
when unmarshal the json file.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
@hqhq
Copy link
Contributor Author

hqhq commented Feb 23, 2016

@wking Using quotes so people know that's a string, after a second thought, I think backticks won't cause confusion, and it's more consistent with other usage in specs, updated, thanks.

@wking
Copy link
Contributor

wking commented Feb 23, 2016

On Mon, Feb 22, 2016 at 10:36:46PM -0800, Qiang Huang wrote:

… I think backticks won't cause confusion… updated…

9bab930 and ccf3a24 look good to me.

@crosbymichael
Copy link
Member

LGTM

1 similar comment
@mrunalp
Copy link
Contributor

mrunalp commented Feb 29, 2016

LGTM

mrunalp pushed a commit that referenced this pull request Feb 29, 2016
@mrunalp mrunalp merged commit 95e1259 into opencontainers:master Feb 29, 2016
@hqhq hqhq deleted the hq_fix_devices_example branch March 1, 2016 01:02
@hqhq hqhq mentioned this pull request Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants