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

(#223) display diff for nested arrays of objects #224

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

masterzen
Copy link
Contributor

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

  • 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.
  • 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 masterzen force-pushed the bugfix/223-display-diff-for-nested-arrays-of-objects branch from 6fa1e51 to 0f46be9 Compare May 14, 2020 16:24
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
index and descend into those nested structure.
@masterzen masterzen force-pushed the bugfix/223-display-diff-for-nested-arrays-of-objects branch from 0f46be9 to e4c3660 Compare May 14, 2020 18:02
@ahayworth ahayworth closed this Dec 15, 2020
@ahayworth ahayworth reopened this Dec 15, 2020
Copy link
Contributor

@ahayworth ahayworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a few more test cases? I'm thinking specifically of cases where the arrays are not just one element apart, but when they differ more significantly.

@masterzen
Copy link
Contributor Author

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

@github github deleted a comment Dec 17, 2020
@github github deleted a comment Dec 17, 2020
@github github deleted a comment Dec 17, 2020
@github github deleted a comment Dec 17, 2020
@github github deleted a comment Dec 17, 2020
@github github deleted a comment Dec 17, 2020
@ahayworth ahayworth added this to the 2.0.0 milestone Dec 17, 2020
@ahayworth ahayworth removed this from the 2.0.0 milestone Jan 12, 2021
@ahayworth
Copy link
Contributor

@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 ahayworth added this to the 2.1.0 milestone Jan 13, 2021
@masterzen
Copy link
Contributor Author

@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.

@ahayworth
Copy link
Contributor

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

@ahayworth ahayworth modified the milestones: 2.1.0, 2.2.0 Feb 18, 2021
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.

Octocatalog-diff is missing diffs from some nested arrays of objects
3 participants