Skip to content
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

Fix the version check for modern node versions. #80

Closed
wants to merge 1 commit into from

Conversation

nicks
Copy link

@nicks nicks commented Jan 21, 2015

Fixes #79

@bajtos
Copy link

bajtos commented Jan 22, 2015

FWIW, this is a duplicate of #78. The approach presented here is more robust since it uses semver instead of regex. On the other hand, it adds an extra dependency to the project.

@nicks
Copy link
Author

nicks commented Jan 22, 2015

ya, i saw that. It seemed weird to assume that all node versions will be 1.x.x or 0.1x.x. What if there's an io.js version 2? or a nodejs version 0.20?

@bajtos
Copy link

bajtos commented Jan 23, 2015

Frankly, I don't care which pull request ends up being landed. The most important for me is to get a fixed version of ncp in npmjs ASAP.

/cc @AvianFlu

@shakiba
Copy link

shakiba commented Jan 25, 2015

+1 for this one!

@chucksellick
Copy link

Version sniffing seems a fragile way to check this kind of thing. How about just checking for the existing of setImmediate? (Not specifically related to ncp, but might it be a good idea for node/iojs to actually provide an api to test for feature existence in cases where significant functionality is introduced between versions? This would allow a robust mechanism to switch behaviour where back compat is required. And another strategy would be to branch ncp to maintain a pre-0.10 version and a post-0.10 version, and specify the compatibility in package.json accordingly. npm could even be able to locate the correct version for the installed engine.)

@bajtos
Copy link

bajtos commented Jan 26, 2015

to branch ncp to maintain a pre-0.10 version and a post-0.10 version, and specify the compatibility in package.json accordingly. npm could even be able to locate the correct version for the installed engine.

👍 ❌ 💯

Considering that v0.10 has been around for almost two years now, I believe we can afford to stop supporting v0.8 in new releases. My argument is that people running on Node v0.8 must be extremely conservative, most likely they won't want to make unnecessary changes to their running system, and thus they won't want to upgrade ncp to a newer version either.

We should consider releasing a new major version (2.0) after we drop support for v0.8.

@chucksellick
Copy link

I agree; and we already have "engines" to safeguard. (+ advantage it's easy to specify node.js and io.js versions independently). Anyone upgrading modules (esp. major version) carelessly is going to get bit sooner or later anyway...

@capaj
Copy link

capaj commented Feb 17, 2015

@chucksellick 👍 probably would be good to get going with the solution you proposed with two forks. A lot of stuff is broken now due to this one lame regex.

@bajtos
Copy link

bajtos commented Feb 18, 2015

Considering the unresponsiveness of project maintainers, it may be worth switching to cpr instead.

This was referenced Feb 18, 2015
@Jonahss
Copy link

Jonahss commented Mar 23, 2015

Just a warning to others, cpr isn't a drop-in replacement for ncp. Copying single files works differently.
Will this get merged?

@nicks nicks closed this Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ncp does not work on io.js due to version sniffing
6 participants