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

Modernized svg-font-dump #5

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

renarsvilnis
Copy link

@renarsvilnis renarsvilnis commented Dec 1, 2016

Hi,
came accross your module today, it did what I needed it to do. Thanks for it!

But there was a DeprecationWarning: Calling an asynchronous function without callback is deprecated. due to the module not being updated last couple of years. After inspecting source, I found out that fs.writeFile without a callback. I forked & cloned the repo and fixed the issue and also did a refactor of the entire module and here is a list of what I did:

  • Migrated from jshint to eslint using semistandard config
  • Extracted helper functions into individual files
  • Dropped lodash in favour of ES6 features which is supported in current LTS node and beyond
  • Rewrote fs operations in async which resulted glyph write operations to run concurrently. Thus performance gains (not really needed, but still)
  • Separated the module into 2 parts - cli and module. Now you can use it as before through cli but also if needed call it programmatically require('svg-font-dump').
  • Renamed cli arguments lower dash (_) symbols to dash (-) symbols some_argument => some-argument make it more consistent as other cli applications Only breaking change for existing users
  • Updated make file to reflect linter change
  • Updated README to reflect changes

Suggest bumping the module to v2.0.0.

Hope the changes are welcomed! 👍

@renarsvilnis
Copy link
Author

renarsvilnis commented Dec 15, 2016

bump 🔨

@bastien-roucaries
Copy link

Ping

@puzrin
Copy link
Member

puzrin commented Jul 11, 2019

Sorry, this project is critical for old-fashioned fontello fonts repos, and need to be in frozen state. Until someone find time (never) to fix all that chaos. Please, do a fork.

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.

None yet

3 participants