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

chore(deps): upgrade deps and use ^ #93

Merged
merged 1 commit into from
Feb 7, 2017
Merged

Conversation

kentcdodds
Copy link
Collaborator

What: chore(deps): upgrade deps and use ^

Why: I've noticed build breakages recently. I believe that this will fix it. Maybe...

How: By hand mostly...

I've long held that locking down deps to a specific version was a good
idea. But now I'm thinking that it's not worth the cost (also it doesn't
really take care of transitive deps anyway).

I've long held that locking down deps to a specific version was a good
idea. But now I'm thinking that it's not worth the cost (also it doesn't
really take care of transitive deps anyway).

I've noticed build breakages recently. I believe that this will fix it.
Maybe...
Copy link
Collaborator

@tleunen tleunen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm if build passes.

Yeah. I also think the same way. Especially with the yarn lock or npm shrinkwrap, locking the versions are not really necessary.

Also, with a greenkeeper setup, it's a bit easier to see when something's wrong in a dependency and requires hot patch/lock

@sxn
Copy link
Collaborator

sxn commented Feb 6, 2017

Same with me: I'm ok with the change if it helps with the builds :-) I see the build's failed again though. 🐼

@tleunen
Copy link
Collaborator

tleunen commented Feb 6, 2017

Yeah... :/ Why is bluebird required again?
Isn't Promise supported in Node 4?

@kentcdodds
Copy link
Collaborator Author

I have no idea what's going on here. I've been able to reproduce it locally a few times, but it's pretty inconsistent. I'm going to go ahead and merge this.

Note that if #94 is merged, this wont matter anymore because the failing test will be removed ;-)

@kentcdodds kentcdodds merged commit 82fa526 into master Feb 7, 2017
@kentcdodds kentcdodds deleted the pr/upgrade-deps-again branch February 7, 2017 16:14
@kentcdodds
Copy link
Collaborator Author

Oh, also, I can't remember whether this package was guaranteed to work on Node 0.12.x but I think that's why I used bluebird. We can probably drop support in the next major version (which is definitely going to happen if #94 is merged).

@kentcdodds
Copy link
Collaborator Author

Oh, and I did set up greenkeeper. I've tried it before and it was too noisy, but we'll try it again and see how we feel about it :)

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.

10 participants