coinbase / terraform-landscape

Improve Terraform's plan output to be easier to read and understand
Apache License 2.0
1.59k stars 116 forks source link

Sort JSON before generating diff #57

Closed randywallace closed 6 years ago

randywallace commented 6 years ago

This PR does something that brings landscape into the realm of total perfection.

Problem: JSON objects are not sorted before compared for diff.

Example: A huge ECS Task Defintion with one minor change resulting in a diff with lots of additions and removals as a consequence of out-of-order data.

Solution: Install deepsort gem and pre-sort parsed JSON before pretty printing and diff.

This worked really well for me, but I haven't updated any of the specs (so this fails tests) because.. it looks like a lot of editing of static fixtures, which I don't have the energy to do.

If you insist, though, I could bring myself to get the existing specs to pass (and probably in a better way, too).

FernandoMiguel commented 6 years ago

Uhhhh that would be nice

randywallace commented 6 years ago

Ok, I fixed the spec. Was easier than I expected.

randywallace commented 6 years ago

Will revise to pull a proper deepsort function to remove the external deepsort dependency with rspec tests. I'll take a couple hours to do it in the next couple days

randywallace commented 6 years ago

@sds Requested review items addressed.

terraform-landscape$ bundle exec rubocop
Inspecting 18 files
..................

18 files inspected, no offenses detected

terraform-landscape$ bundle exec rspec
.................................

Finished in 0.30581 seconds (files took 0.21665 seconds to load)
33 examples, 0 failures

Please advise if you would like me to squash these two commits; I didn't as squashing mangles review comments and whatnot.