Skip to content

Conversation

@zenlint
Copy link
Contributor

@zenlint zenlint commented Aug 10, 2015

Correct err in devices json in config-linux.md, type field should be top of the path field, according to the spec_linux.go.

Signed-off-by: LinZhinan(Zen Lin) [email protected]

…top of the path field, according to the spec_linux.go.

Signed-off-by: LinZhinan(Zen Lin) <[email protected]>
@wking
Copy link
Contributor

wking commented Aug 10, 2015

On Mon, Aug 10, 2015 at 04:51:27AM -0700, LinZhinan(Zen Lin) wrote:

Correct err in devices json in config-linux.md, type field should be
top of the path field, according to the spec_linux.go.

I don't think the key order actually matters for JSON (so it's
probably not an error as it stands). But I think we might as well be
consistent in our ordering anyway, so I'm +1 on this change (I'd also
be fine with reordering the fields in spec_linux.go).

@zenlint
Copy link
Contributor Author

zenlint commented Aug 11, 2015

@wking Yes, so agree with u, maybe I have mistook to exp it as a error,thanks.

@zenlint
Copy link
Contributor Author

zenlint commented Aug 11, 2015

@wking To your opinion, which file to be changed is more reasonable, spec_linux.go or config-linux.md.
If change the order in spec_linux.go ,I am going to change the PR.

@philips
Copy link
Contributor

philips commented Aug 11, 2015

path seems like the "unique key" in this struct so I guess we should reorder the spec_linux.go instead.

@zenlint
Copy link
Contributor Author

zenlint commented Aug 11, 2015

@wking @philips I have newed a PR for changing it in the spec_linux.go file.
Change Device field order in spec_linux.go #106

@zenlint zenlint closed this Aug 11, 2015
@wking
Copy link
Contributor

wking commented Aug 11, 2015

On Mon, Aug 10, 2015 at 09:08:22PM -0700, Brandon Philips wrote:

path seems like the "unique key" in this struct so I guess we should
reorder the spec_linux.go instead.

I believe path is optional now that this structure also handles
cgroups rules (#94, proposed docs in #102). So it's not a unique key.

I really have no preference for which order we chose ;).

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.

3 participants