jspm / sver

Simple Semver and SemverRange classes
MIT License
16 stars 4 forks source link

Mind me forking for node <6? #1

Closed phated closed 7 years ago

phated commented 7 years ago

Hey @guybedford, this project is awesome! I stumbled upon it while exploring improvements for https://github.com/gulpjs/semver-greatest-satisfied-range and it looks like you already solved some of my problems. Would you mind if I forked to add node <6 support to be used in gulp-cli?

guybedford commented 7 years ago

Hey, I'm really glad to hear the project is of use to you, and thanks for letting me know! Definitely go ahead, and keep me posted how it goes - maybe we can merge in due course as well.

phated commented 7 years ago

@guybedford you rock! Any thoughts on what be causing my one failure at https://travis-ci.org/phated/sver-compat/jobs/265350661?

guybedford commented 7 years ago

@phated well I guess I do still owe you a beer from JS Conf all those years ago :P I just had a look at it briefly, and I think it might be the conversion of the two loops from https://github.com/guybedford/sver/commit/3d8c7fbed52cc75f36e3eb9c54f0d8f53fb022fe#diff-98882b3be0c3bec6da6c8b0e69e40283R28. The for in will loop over the indices so it would be seen as "0 1 2 3", and hence the =3.0.0 conversion. But I haven't tested it out myself so may just be making this up, but it sounds plausible enough.

phated commented 7 years ago

@guybedford perfect! I've switched to a forOf pseudo-shim (https://github.com/phated/sver-compat/commit/551419f864e7bf75416926d77d1354b8f2c3093f) and it works 😄

phated commented 7 years ago

Thanks again! I'd say it's a pretty clean compat version based on my merge at https://github.com/phated/sver-compat/commit/4d50b542f86d67d8be40c80cd817301cea3f2322

Let me know if you ever want to merge these, as I'll be using sver-compat in gulp-cli 😄