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

Remove explicit CLS dependency #75

Merged
merged 1 commit into from
Jan 12, 2016
Merged

Conversation

hildjj
Copy link
Contributor

@hildjj hildjj commented Nov 19, 2015

As suggested in #70. People that want to use CLS can take the dependency themselves.

@analog-nico
Copy link
Member

Hi @hildjj, a thought I had, too. However, since removing the dependency is a breaking change I would rather work on the upcoming v3 release: It will add ES6 promise support (issues #65 and #71) which most likely will allow the user to inject his preferred promise implementation. With that the CLS support can be configured outside of Request-Promise and bindCLS as well as the dependency can be dropped.

Does this sound good to you? The release will take a few weeks but then the breaking changes come with new features. Until then I would keep this PR open so I can merge it before the release.

@hildjj
Copy link
Contributor Author

hildjj commented Nov 19, 2015

Version numbers are cheap, but I understand. I can depend on my fork until you're done.

@jey
Copy link

jey commented Dec 30, 2015

👍.

Currently, npm install request-promise warns:

npm WARN EPEERINVALID cls-bluebird@1.0.1 requires a peer of continuation-local-storage@~3 but none was installed.

@ekryski
Copy link

ekryski commented Jan 8, 2016

Ya I'm with @hildjj. Version numbers are cheap. If we can cut a new major release without CLS that would be sweet. 😁

@analog-nico
Copy link
Member

Hey guys, I am with you. Indeed, Request-Promise is in a bad state with that. Just give me a few days. I am really busy right now.

analog-nico added a commit that referenced this pull request Jan 12, 2016
Remove explicit CLS dependency
@analog-nico analog-nico merged commit 05b6314 into request:master Jan 12, 2016
@analog-nico
Copy link
Member

I just published request-promise@2.0.0 which doesn't depend on cls-bluebird anymore and thus also doesn't fail npm shrinkwrap anymore. Thanks @hildjj for the PR!

@ekryski
Copy link

ekryski commented Jan 12, 2016

Woot! Thanks @analog-nico!

@analog-nico
Copy link
Member

Hi buddies, I just releases request-promise@4.0.0 which doesn't call require('cls-bluebird') at all anymore. That means that you may now remove any workarounds – e.g. for your resource bundler – you may still have in place.

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.

4 participants