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 winston dependency #12

Merged
merged 2 commits into from
Dec 30, 2013
Merged

Remove winston dependency #12

merged 2 commits into from
Dec 30, 2013

Conversation

RasterBurn
Copy link

Previously, winston-logstash would install a version of winston into
node_modules/winston-logstash/node_modules/winston. If node chooses to
use this version of winston rather than the cached version from
node_modules/winston, the consuming application's winston would not have
the Logstash transport attached to it.

This fix removes winston as a dependency, and it is assumed that winston
must be installed by the consumer.

Previously, winston-logstash would install a version of winston into
node_modules/winston-logstash/node_modules/winston. If node chooses to
use this version of winston rather than the cached version from
node_modules/winston, the consuming application's winston would not have
the Logstash transport attached to it.

This fix removes winston as a dependency, and it is assumed that winston
must be installed by the consumer.
@RasterBurn
Copy link
Author

I also suppose peerDependencies would also work in this situation.

@cjsaylor
Copy link
Contributor

👍

@RasterBurn
Copy link
Author

I added winston back as a peer dependency. If you approve, could you merge it and release to npm?

jaakkos added a commit that referenced this pull request Dec 30, 2013
Remove winston dependency
@jaakkos jaakkos merged commit ce8a31b into jaakkos:master Dec 30, 2013
@jaakkos
Copy link
Owner

jaakkos commented Dec 30, 2013

Thanks, I have been offline during holidays :)

@jaakkos
Copy link
Owner

jaakkos commented Dec 30, 2013

Published new version https://npmjs.org/package/winston-logstash

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.

3 participants