-
Notifications
You must be signed in to change notification settings - Fork 32
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
Cool idea #3
Comments
Thanks @cscott. I'll take a look. |
I like @cscott's use of
So I'm hesitant to recommend this tool yet. I landed here trying to answer the question of whether it makes sense to recommend using this tool via the node-pre-gyp README (pre mapbox/node-pre-gyp#108 (comment)). |
@springmeyer Thanks for getting back to me on this. It makes sense to add each of those items, so I will do so. Task List:
|
great @bchr02. The simplicity of https://github.com/cscott/node-icu-bidi/blob/master/scripts/publish.js is also appealing so you might look for other smart things it does? |
okay, I will do that. |
@springmeyer with 73e8be3 I have just added unit tests with over 97% line coverage. I still plan on adding CI and reveal, but maybe now this is enough for you to recommend this module. 😄 |
@springmeyer now at 100% coverage! |
Thanks @bchr02 - Yes, it is. I'm traveling - perhaps you could make a PR against node-pre-gyp adding mention and usage to the readme? |
@springmeyer Sorry for taking so long. As you requested, I have made mention and submitted mapbox/node-pre-gyp#216 Thank you. |
I've been doing something similar for a while; see:
https://github.com/cscott/node-icu-bidi
https://github.com/cscott/node-php-embed
etc.
Perhaps you can borrow some ideas from my scripts? I've been too lazy to actually package everything up as a nice standalone npm packages, glad to see you're doing that.
The text was updated successfully, but these errors were encountered: