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

Upgrade npm package dependencies #100

Merged

Conversation

PeterDaveHello
Copy link

@PeterDaveHello PeterDaveHello commented Nov 6, 2017

Most of the devDependencies were dated, ref: https://david-dm.org/notatestuser/gift?type=dev

@notatestuser maybe we should dropped end-of-life nodejs versions like v0.10 & v0.12?

@PeterDaveHello
Copy link
Author

@notatestuser maybe we should dropped end-of-life nodejs versions like v0.10 & v0.12? I doubt if modern packages still supports nodejs > 0.4 < 0.12, maybe we can bump the nodejs engine in package.json and .travis.yml to drop the deprecated versions?

@PeterDaveHello PeterDaveHello force-pushed the upgrade-npm-dependencies branch from 000f638 to 0fcec74 Compare November 6, 2017 12:50
@PeterDaveHello PeterDaveHello force-pushed the upgrade-npm-dependencies branch from 0fcec74 to d8cd1a2 Compare November 6, 2017 12:53
@notatestuser
Copy link
Owner

I agree but dropping old node would be more suitable for a v1.0 release. Does updating dev dependencies make the tests break on those versions?

@PeterDaveHello
Copy link
Author

PeterDaveHello commented Nov 6, 2017

I think it passed the tests on all versions after v4, the issue here is we can't run the test, not the test failed.

Not only mocha but also sinon uses supports-color which needs nodejs >4.0

Not sure what's the version bumping policy here, I think the old dependencies stocked too long, so maybe it's the chance to bump to v1.0.0? (During a long time from the previous release)

BTW, I don't think it means it breaks the using on old versions, just can't run test on them, and it won't violate what the readme says test on all stable versions :)

@PeterDaveHello
Copy link
Author

@notatestuser sure thing!

@notatestuser notatestuser merged commit d42dc58 into notatestuser:master Nov 6, 2017
@notatestuser
Copy link
Owner

Actually I'll do it, no worries. Thanks again!

PeterDaveHello added a commit to PeterDaveHello/gift that referenced this pull request Nov 6, 2017
@PeterDaveHello
Copy link
Author

Oops ... I just pushed one second later ...

@PeterDaveHello
Copy link
Author

@notatestuser and I think node in package.json should be updated to >4 please.

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.

2 participants