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

Added a Sass version with build test using Gulp #4

Merged
merged 1 commit into from
Apr 13, 2018

Conversation

emileber
Copy link
Contributor

@emileber emileber commented Sep 26, 2016

This closes #3.

I applied the change I proposed in #2 only to the .scss file, concerning a clash with other font libs using icon- as a default prefix.

I added default variables inspired by simple-line-icon to enable changing:

  • the path of the icon directory,
  • the prefix,
  • and the font-family name if someone would want to provide his own font files.

I also added a precommit hook which prevent committing a .scss file that can't be built (at least).

There's a .travis.yml file which could serve the same test server-side to ensure each commit and pull request meets the requirements.

More could be added to this, like a task to build the fonts from the src folder, etc.

@emileber
Copy link
Contributor Author

Oh, and also, since I'm using a package.json file to keep track of the dev dependencies, you could publish this to npm.

I already bumped both package.json and bower.json files to 1.1.0 as its a lot of new features, but no breaking changes.

@jamesadevine
Copy link
Owner

@emileber This looks great, sorry it took a while for me to get back to this.

The existing API is untouched which is great, and if it can be used immediately with npm, that sounds like an immediate win.

I hear bower is "less cool" now anyway 😉

Great idea with the custom prefix BTW 👍

Travis is a nice direction, unless this project gets super big I don't feel the need for it quite yet.

I assume this is tested through and it works?

@jamesadevine
Copy link
Owner

(I will double check on my side before merging)

@emileber
Copy link
Contributor Author

emileber commented Oct 11, 2016

I personally use both npm and bower. npm is usefull for everything, but I like splitting my backend dependencies and my frontend dependencies using the right tool for each, bower being best for frontend lib.

The best is to avoid imposing choices to users by providing both.

You're right, Travis might be overkill for a small project like this one, but it's free for open-source project and it's easy to activate, just toggling a checkbox.

This is tested, I'm using my own fork in an big enterprise app which uses Sass until this is merged.

@jimmykane
Copy link

Any update on this ?

@emileber
Copy link
Contributor Author

@jimmykane James looks busy, so if you really need the sass version, you can use the branch sass on my fork:

npm install [email protected]:emileber/sportsfont.git#sass --save

@jamesadevine
Copy link
Owner

jamesadevine commented Apr 13, 2018

@emileber busy is definitely what I am, however I've been disregarding my duties as a maintainer, and disrespecting you and your contribution by not merging it for a long time.

I'm happy to push the button now --- is there anything I need to do?

@jamesadevine jamesadevine merged commit 9bff8c9 into jamesadevine:master Apr 13, 2018
@emileber
Copy link
Contributor Author

emileber commented Apr 13, 2018

If you want, you can toggle the checkbox on Travis to make sure the files are compiling fine. It's free for open-source projects and the configuration file (.travis.yml) is already included in this PR.

image

@emileber emileber deleted the sass branch April 13, 2018 14:33
@emileber
Copy link
Contributor Author

Also, it would be useful to push another version to NPM and tag it in the repo as well.

@jamesadevine
Copy link
Owner

@emileber Thank you for your contribution, and sorry for ignoring you for so long. It's now on npm! 🎉

@emileber
Copy link
Contributor Author

You're the real MVP, thanks for the open source font plugin ❤️

@jimmykane
Copy link

@jamesadevine @emileber Thanks a ton guys. I can now implement these activity icons on my project. You are both the best! 😘

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.

Add a sass version to easily import and customize
3 participants