mrodrig / json-2-csv

Convert JSON to CSV *or* CSV to JSON!
https://mrodrig.github.io/json-2-csv
MIT License
421 stars 58 forks source link

chore: change deprecated string method #258

Closed diegoreis42 closed 4 months ago

diegoreis42 commented 4 months ago

Background Information

I have...

While reading the code, I found deprecated warnings. This is not a breaking change, but rather a precaution against potential errors. A clean project like this should not have trivial and annoying warnings.

mrodrig commented 4 months ago

Thanks for the PR @diegoreis42! I hadn't noticed the deprecation warnings recently, so that's a good catch.

It looks like the tests are failing currently for these changes, and I believe it's because the parameters for substr and substring are slightly different. MDN has some details about the differences here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring#the_difference_between_substring_and_substr

image

(Edit: Added a screenshot since the link to the specific section wasn't working)

If you're able to update the logic accordingly, I'd be happy to get this merged in and publish a new version with this update.

Thanks again!

diegoreis42 commented 4 months ago

I'm sorry for the mistake; I only ran the tests for my first change 😅. All tests are now passing.

coveralls commented 4 months ago

Coverage Status

coverage: 98.015%. remained the same when pulling fd1871f1c0214e9eb2ba370f9a793d95ac744d6b on diegoreis42:minor-fix into 84f088b944a0d26ea4ae7c082439162d9047150e on mrodrig:main.

mrodrig commented 4 months ago

Thanks @diegoreis42! I'll get this published in 5.5.3 now.