search

smhg / gettext-parser

Parse and compile gettext po and mo files, nothing more, nothing less
MIT License
158 stars 44 forks source link

Add `readable-stream` to the dependencies to fix an error with `stream` module #42

Closed coolstuffit closed 5 years ago

coolstuffit commented 5 years ago

Something wrong with the node version. It uses 0.10

smhg commented 5 years ago

Thank you for your contribution! I'm ok with using readable-stream. But you'd need to make the tests work. I scanned their readme: it seems version 2 is what you need to use. Can you give this a try?

RignonNoel commented 5 years ago

@smhg Can i help on this one ? I really need this fix to work on translation with Angular6.

When i checked the Travis it seems that this PR is valid since Node 0.10 and 0.12 are deprecated since 2016. What do you want us to do to merge this fix ?

Documentation Node.Js : https://github.com/nodejs/Release#end-of-life-releases

smhg commented 5 years ago

@RignonNoel I'm fine with dropping support in a new major version. In the meanwhile though, the 'fix' for this PR seems easy? Please correct me if I'm wrong. If so, it sounds best to do small release and do a major version with some other features (and/or ES6 rewrite).

RignonNoel commented 5 years ago

@smhg Yes the fix it's pretty easy, my concern is more about the support of the version 2 since readable-stream write no one documentation about it. The version 3 is 7 months old and i really don't know if the team will support security issue in version 2.

You're right, you can just accept the PR #46 i created yesterday and wait before push a V3 if one day a security issue is discovered (or you added some new features), it's just that your lib will be a leak of security between the moment the leak is discovered and the moment you change your requirement to a supported version of readable-stream, can be bad for users.

smhg commented 5 years ago

Closing because this was fixed in #46.

smhg commented 5 years ago

@RignonNoel I've also released version 3, which removes old node support. That should fix any current security issues (in dev mode).

RignonNoel commented 5 years ago

@smhg thanks for your help on this! And thanks for this great lib and all the work invest in it :)