octocatalog-diff icon indicating copy to clipboard operation
octocatalog-diff copied to clipboard

(#223) display diff for nested arrays of objects

Open masterzen opened this issue 5 years ago • 4 comments

Overview

This pull request fixes #223.

When using octocatalog-diff with puppet resources with deep nested datastructures such as nested arrays of objects, octocatalog-diff would not display diffs when array elements are modified, added or removed.

In fact, it turns out dig_out_key doesn't descend into arrays index that hashdiff can produce, like for instance: testtype::test::parameters::testparam::another-one::an-array[0]::env

dig_out_key would stop at an-array because it doesn't know that's and array index it should try to descend into.

This patch adds the functionality for dig_out_key to follow array indices and descend into those nested structures.

Checklist

  • [x] Make sure that all of the tests pass, and fix any that don't. Just run rake in your checkout directory, or review the CI job triggered whenever you push to a pull request.
  • [x] Make sure that there is 100% test coverage by running rake coverage:spec or ignoring untestable sections of code with # :nocov comments. If you need help getting to 100% coverage please ask; however, don't just submit code with no tests.
  • [ ] If you have added a new command line option, we would greatly appreciate a corresponding integration test that exercises it from start to finish. This is optional but recommended.
  • [ ] If you have added any new gem dependencies, make sure those gems are licensed under the MIT or Apache 2.0 license. We cannot add any dependencies on gems licensed under GPL.
  • [ ] If you have added any new gem dependencies, make sure you've checked in a copy of the .gem file into the vendor/cache directory.

/cc #223

masterzen avatar May 14 '20 16:05 masterzen

@ahayworth , I'll do my best to add a few more tests.

masterzen avatar Dec 16 '20 11:12 masterzen

@masterzen - if you're able to add a few more specs, I'd like to target this for 2.1.0 at the end of this month. If you're not able to do so right now, just let me know - we may be able to make some time on our end to help out as well. 😄

ahayworth avatar Jan 13 '21 15:01 ahayworth

@ahayworth I'm very sorry, I thought I would be able to do that during the holidays, but alas I couldn't spare a minute. Unfortunately, I don't think I'll be able to add those tests before the 2.1.0 deadline :( Thanks for your help on this topic.

masterzen avatar Jan 13 '21 17:01 masterzen

Thanks! I'll see if I can work it in on my end. 😄

ahayworth avatar Jan 13 '21 20:01 ahayworth