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 deps #46

Merged
merged 2 commits into from
Aug 15, 2017
Merged

Upgrade deps #46

merged 2 commits into from
Aug 15, 2017

Conversation

SimenB
Copy link
Collaborator

@SimenB SimenB commented Aug 15, 2017

This upgrade all deps to latest.

It also adds ^ to dependencies so that they can be deduped for consumers.

(breaking change in chalk@2 is removal of stripColors and support for old nodes)

@SimenB SimenB requested a review from af August 15, 2017 14:14
@af
Copy link
Owner

af commented Aug 15, 2017

I'm generally not a big fan of ^ for dependencies, since library authors are human and non-semver-compliant changes can break things. Could you pin the deps to strict versions? Other than that this looks good, some of those deps are really old

@SimenB
Copy link
Collaborator Author

SimenB commented Aug 15, 2017

My thought was that with yarn and npm@5 people use lockfiles. It moves the responsibility over to consumers to lock down deps, but also empowers them to dedupe deps to achieve smaller bundles/apps.

That said, I don't really feel strongly about this. But maybe activate Greenkeeper so we don't drift too much?

@af
Copy link
Owner

af commented Aug 15, 2017

That is a very valid point about lockfiles, everyone should be using them at this point so floating dep versions is less of a problem these days. Still, pinning to exact versions lessens our burden of ensuring envalid works the matrix of matching dependency versions. Using ^ advertises that this lib will work with all of the matching versions, which we can't ensure without testing them all.

@af af merged commit e28adf6 into af:master Aug 15, 2017
@SimenB SimenB deleted the upgrade-deps branch August 15, 2017 16:09
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.

None yet

2 participants