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

Fix 168 #196

Closed
wants to merge 2 commits into from
Closed

Fix 168 #196

wants to merge 2 commits into from

Conversation

mrodrig
Copy link
Owner

@mrodrig mrodrig commented Jun 1, 2021

Background Information

I have...

  • added at least one test to verify the failure condition is fixed.
  • verified the tests are passing.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.266% when pulling f1f7408 on fix-168 into bf4dd22 on stable.

@coveralls
Copy link

coveralls commented Jun 1, 2021

Coverage Status

Coverage remained the same at 98.276% when pulling 7424e7a on fix-168 into d754925 on stable.

@mrodrig
Copy link
Owner Author

mrodrig commented Jun 1, 2021

To do:

  • Update README
  • Check best way to handle deeks module option specification

In order to convert the generated CSV back to the most accurate JSON
possible, the module must still insert `[]` for unwound empty array
values. However, there are some legitimate cases where users want to be
able to have the module ignore these values so that the generated CSV is
a bit more user-friendly, depending on the data being converted. This
new option allows users the ability to achieve this without impacting
existing use cases, as the user has opted to specify an option which may
impact the ability to convert the CSV back to JSON later on.

Fixes #168
@@ -656,15 +656,24 @@ function runTests(jsonTestData, csvTestData) {
});
});

// Test case for #168

Choose a reason for hiding this comment

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

👍👍

@mrodrig mrodrig changed the base branch from stable to main February 18, 2023 05:52
@mrodrig
Copy link
Owner Author

mrodrig commented Feb 25, 2024

Closing as this will require revisiting anyway.

@mrodrig mrodrig closed this Feb 25, 2024
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.

None yet

3 participants