wizicer / gulp-csvtojson

csv to json converter for gulp
MIT License
3 stars 1 forks source link

Handle Streams #1

Closed tjchaplin closed 8 years ago

tjchaplin commented 8 years ago

The current plugin doesn't handle streams. Ideally Gulp plugins should handle the three types of vinyl:

https://github.com/gulpjs/gulp/blob/master/docs/writing-a-plugin/README.md

tjchaplin commented 8 years ago

The pull request has updated tests, and should properly handle streams now.

Would this be ok to merge in? Do you want to discuss further?

wizicer commented 8 years ago

Wonderful work, you almost rewrite the whole code base, I'll look into it ASAP.

BTW: I just observed one unit test failed in your PR.

tjchaplin commented 8 years ago

Please let me know. All unit tests passed on my side. I have also upgraded the packages, so make sure to do an npm install before npm test.

wizicer commented 8 years ago

Finally, I changed replace(/\s\n/g,'') to replace(/\s?\n/g,'') to make it work, my test bed is linux which only generate \n.

wizicer commented 8 years ago

code merged, so close this issue.