berkshelf / ridley

A reliable Chef API client with a clean syntax
Other
231 stars 85 forks source link

Use pretty json format for metadata.json #287

Closed iroller closed 9 years ago

iroller commented 9 years ago

Chef suggests using knife diff for comparing local repo and chef server.

To compare cookbooks uploaded with ridley we need to compile metadata.json first.

There's already a knife command for it: knife cookbook metadata from file metadata.rb

It generates metadata.json using pretty json format. So I suggest generating pretty jsons for metadata.rb when uploading so we can use knife diff against it and see nice diffs.

Note: There's still a difference between ridley's and knife's JSON.pretty_generate since knife is using https://github.com/opscode/ffi-yajl. yajl/json_gem adds an extra new line in empty hashes and json doesn't:

[1]pry(main)> require 'yajl/json_gem'
true
[2]pry(main)> JSON.pretty_generate(a: {})
"{\n  \"a\": {\n\n  }\n}"

[3]pry(main)> require 'json'
true
[4]pry(main)> JSON.pretty_generate(a: {})
"{\n  \"a\": {\n  }\n}"

Not really sure how to deal with the difference in how json and yajl's json are generating jsons. Only found one difference so far, might be not a big problem. Added an issue for yajl-ruby as well: https://github.com/brianmario/yajl-ruby/issues/150

reset commented 9 years ago

@iroller the real problem here is that the Chef server is passing us back the metadata as we formatted it. Even with this change we can't guarantee that Knife changes the way it's formatting things causing us to break again.

This should be opened as an issue with Chef Server and hopefully it'll be addressed in a future release.

I'll accept this for now, but please add a comment to the change noting this ticket and why we're using pretty generate instead of fast generate.

iroller commented 9 years ago

@reset I agree. Also seems like chef shouldn't use line-by-line diff for jsons. Added a comment.

iroller commented 9 years ago

Not sure why Travis is failing now since I added a comment. Looks fine locally:

$ thor spec:all
...
Finished in 24.43 seconds
458 examples, 0 failures, 10 pending
reset commented 9 years ago

@iroller nah it's fine

reset commented 9 years ago

@iroller please follow this up with a ticket for the Chef Server and link it here!

iroller commented 9 years ago

Thanks :+1:

Added a followup to chef repo: https://github.com/opscode/chef-server/issues/28