-
Notifications
You must be signed in to change notification settings - Fork 41
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
v2.5.2 #66
Conversation
fixes the issue with empty strings in nested arrays getting filled by the next array item also includes a minor refactoring of fillRows function
Y’all I’ll check what’s going on here within 5 days. I’ll set a reminder now. With next couple days off, I might get to this a lot sooner. I just now remember we had a true issue reported and maybe these recent changes fix that PLUS I’ll rebuild and touch up the web demo |
@AckerApple nice! thanks :) About the true issue, that's probably about #65 #57 #22, all of those should be fixed by #68 (i added a new test to check for this problem) Acker try to setup an IM so we can chat a bit :) maybe gitter? https://gitter.im/jsonexport/Lobby |
…ring bugfix nested arrays with empty strings filled by the next array item
@AckerApple I dont get it why you went ahead and merged this 40987ac into master? i had a different thing in mind, i was going to release a minor version with security patches, then those fixes into a major version bump since we introduced a braking change in the csv output. The PR your merged is also related with #68 i would prob release both together Also, looks like your merge stripped the commit original author and squashed my commits https://github.com/kauegimenes/jsonexport/commits/master |
My thought and intention was to release just your fix by itself as a minor fix. And then allow everything to come after. I do apologize |
Hey soooo going forward any requests? I won’t touch anything going forward at this time without your input. Please do communicate your thoughts for moving forward. Again I do apologize. |
@AckerApple i will prob cherry pick the npm vulnerabilities patches and release a minor version. Lets keep working on a few issues and prepare the 3.0.0 release in the Are you planning to work on #53 this week? I think it would be a great addition to update the demo page for 3.0.0. |
Acknowledged. I figured you would do the security stuff as a patch. Great call out on the web options I forgot about that and actually just updated and added features to the demo so this will be super easier. I’ll have it done in under 6 days max but maybe in two I predict |
I have updated the web demo to support most all options. Basically just the simple easy ones to implement. You will instantly see the results of options using this link here |
rm stream memory limit check
@AckerApple nice! :D |
Status
READY
Description
This will be a major version bump since #68 introduces a breaking change.
Related PRs
Waiting for #68 #67 #70
Todos
npm audit
for this branch