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 peer dependencies #32

Merged
merged 3 commits into from
Oct 6, 2015
Merged

remove peer dependencies #32

merged 3 commits into from
Oct 6, 2015

Conversation

nfriedly
Copy link
Contributor

@nfriedly nfriedly commented Oct 3, 2015

Alternative to #31, also fixes #29

(or one of it's sub-modules) now that npm won't enforce that for us.
@backflip
Copy link
Owner

backflip commented Oct 3, 2015

@nfroidure Since peer dependencies do not exist anymore: Would you specify gulp-iconfont as a "real" dependency?

@nfriedly
Copy link
Contributor Author

nfriedly commented Oct 3, 2015

I think people will have to keep installing it as a separate dependency unless we make significant changes to this module to have it actually use gulp-iconfont internally.

(If the user has to require() it, then it has to be a top-level dependency for the current version of npm.)

@backflip
Copy link
Owner

backflip commented Oct 4, 2015

The advantage would be that we could specify a version range which is compatible with this plugin.

@nfriedly
Copy link
Contributor Author

nfriedly commented Oct 4, 2015

Yea, its really not a bad idea, its just that its more involved than the quick-fix here.

@backflip
Copy link
Owner

backflip commented Oct 4, 2015

Fair enough. But we need it as a devDependencies for the tests to run.

@nfriedly
Copy link
Contributor Author

nfriedly commented Oct 4, 2015

So, now that I'm looking at the code more closely, it might not be so hard to just pipe to gulp-iconfont internally, then we'd have it as a regular dependency and users won't even need to install it. Let me see if I can put it together today or tomorrow...

@backflip
Copy link
Owner

backflip commented Oct 4, 2015

@nfroidure Would you be fine with adapting this plugin to use gulp-iconfont internally or would you rather see it being officially deprecated?

For some context: As mentioned in the README, this plugin is not really necessary. Instead, the glyphs event of gulp-iconfont could be used. This would possibly help preventing issues like nfroidure/gulp-iconfont#73

@backflip
Copy link
Owner

backflip commented Oct 4, 2015

@nfriedly On second thought: Let's not use gulp-iconfont internally. A gulp plugin is supposed to do one thing only and this would kinda work against it. If you add it as a devDependency I will merge it and release a new version. Thanks for your patience. :)

@nfriedly
Copy link
Contributor Author

nfriedly commented Oct 4, 2015

Sounds good, just a minute.

@backflip
Copy link
Owner

backflip commented Oct 4, 2015

No pressure, I will be back in about 8 hours.

Bump gulp-iconfont to v5
@nfroidure
Copy link
Contributor

Free as in freedom ;)

gulp-iconfont bundles: gulp-svgicons2svgfont gulp-svg2ttf gulp-ttf2woff gulp-ttf2woff2 etc...

I'm ok with any use of gulp-iconfont :).

backflip added a commit that referenced this pull request Oct 6, 2015
@backflip backflip merged commit 8286fe6 into backflip:master Oct 6, 2015
@backflip
Copy link
Owner

backflip commented Oct 6, 2015

Okay. :)

@nfriedly: Thanks again, released with 2.0.0.

@nfriedly
Copy link
Contributor Author

nfriedly commented Oct 6, 2015

Awesome, thanks!

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.

peerDependencies issues
3 participants