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

client.JSONPrinter{} should not sort the JSON items #141

Closed
VojtechVitek opened this issue Sep 24, 2014 · 10 comments
Closed

client.JSONPrinter{} should not sort the JSON items #141

VojtechVitek opened this issue Sep 24, 2014 · 10 comments

Comments

@VojtechVitek
Copy link
Contributor

client.JSONPrinter{}, as used in openshift kube process -c template.json command, should preserve order of the template's JSON items for better readability.

Seems like the used json.MarshalIndent() method https://github.com/openshift/origin/blob/master/pkg/cmd/client/json_printer.go#L20 sorts the items automatically. We need to either reimplement this method or figure out how to turn off the sorting.

Also, I'd expect the '\n' character at the end of the result JSON output.

/cc @mfojtik @bparees

@mfojtik
Copy link
Contributor

mfojtik commented Sep 24, 2014

@VojtechVitek I would argue against the trailing newline.

Also are you 100% positive that the MarshalIndent (or the json.Indent()) are screwing the order?

@VojtechVitek
Copy link
Contributor Author

I wouldn't say 100%, but what else would cause it?

@smarterclayton
Copy link
Contributor

Is the order really that bad?

@bparees
Copy link
Contributor

bparees commented Sep 26, 2014

it makes the files pretty hard to read as a user, yeah.

@smarterclayton
Copy link
Contributor

You mean if i'm looking for a field I can actually find it? That sucks...

@bparees
Copy link
Contributor

bparees commented Sep 26, 2014

it's unreadable because it becomes hard to match up fields with the containing object when you've got fields at the bottom of a 30 line nested structure.

it's much better if the simple fields are at the top, before the nested crap, rather than trying to figure out which level of nesting a simple field at the bottom is associated to.

See example here:
https://gist.github.com/bparees/e68c4f2fafeab9be1194

Tell me which object this label applies to:
https://gist.github.com/bparees/e68c4f2fafeab9be1194#file-gistfile1-txt-L79

Or tell me what kind of object this is:
https://gist.github.com/bparees/e68c4f2fafeab9be1194#file-gistfile1-txt-L22

@smarterclayton
Copy link
Contributor

The argument that would convince me would be "we should be returning it in the indented order of the underlying resource object", otherwise, we should return in sorted order for things we have no idea what they are.

@soltysh
Copy link
Member

soltysh commented Sep 26, 2014

Generally speaking JSON fields are out of nature unordered. So preserving some order in most cases will be just unnecessary requirement. The only acceptable solution I see here, is sorting in natural order, this way it's easy to find the fields you're looking for. Besides grep does the trick, my 2 cents ;)

@VojtechVitek
Copy link
Contributor Author

"we should be returning it in the indented order of the underlying resource object"

Yes, that's exactly what I had in my mind. And both k8s and origin already formats the JSON fields order based on the underlying objects. The only exception, for now, is the kube process command.

I think, for readability, it's much better to have "kind" and "id" on the top.

@smarterclayton
Copy link
Contributor

Closing without prejudice - reopen upstream or suggest a fix.

danwinship pushed a commit to danwinship/origin that referenced this issue Jun 24, 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

No branches or pull requests

5 participants