search

rxaviers / cldr-data-npm

Npm module for Unicode CLDR JSON data
MIT License
42 stars 28 forks source link

install: Fix detection of npm@3 on node@0.10 #34

Closed bajtos closed 8 years ago

bajtos commented 8 years ago

The install script uses child_process.execSync to detect npm version. This API was added in Node v0.12 and is not available in Node v0.10. As a result, the script always download CLDR data, even when this package was not explicitly listed in project dependencies.

This commit fixes the problem by using npm_config_user_agent as a fallback mechanism.

Related: https://github.com/strongloop/strong-globalize/issues/58

@rxaviers PTAL

rxaviers commented 8 years ago

Thank you @bajtos, it seems like a good solution to me. I'm only waiting to hear what you think about an inline comment I left.

Also please read https://github.com/rxaviers/cldr-data-npm/blob/master/DCO.md and sign-off.

Thanks

bajtos commented 8 years ago

Also please read https://github.com/rxaviers/cldr-data-npm/blob/master/DCO.md and sign-off.

Ah, that reminds me I need to get an approval from my employer for this. This will take at least several days, sorry for that!

bajtos commented 8 years ago

@rxaviers I have added the comment, read DCO and signed off my commit. Could you PTAL again?

bajtos commented 8 years ago

I have added the comment, read DCO and signed off my commit. Could you PTAL again?

@rxaviers ping

rxaviers commented 8 years ago

Sorry for the lack of response, hadn't find time. Will try to do it ASAP.

bajtos commented 8 years ago

Sorry for the lack of response, hadn't find time.

No worries, I totally understand that!

rxaviers commented 8 years ago

Thank you, published as 29.0.2 (and also published in all other major branches 28.x, 27.x, etc).