-
Notifications
You must be signed in to change notification settings - Fork 29
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
Switch to @gustavnikolaj/async-main-wrap #44
Conversation
Pull Request Test Coverage Report for Build 53
💛 - Coveralls |
The drop in coverage % must be due to the removal of well-tested boilerplate code. |
760aa38
to
90cf9fb
Compare
@Munter, any thoughts? |
There's a new version 3.0.0 that is just some neater output in error cases by default. :-) |
@gustavnikolaj, cool! I updated to that now. |
e1629cd
to
49c66ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can do this. Feels a bit ugly that all the conversion of arguments into cleanly applying options are now in the same file though. I'd prefer if we can keep the args to options conversion separated
Cool, we can think about cutting that differently. As long as we get to make tests for the tool as a whole I’m happy :) |
Implement --dryrun and --silent
49c66ff
to
31b9c07
Compare
As long as |
Gets rid of the arbitrary division of labor between lib/cli.js and lib/subfont.js and allows testing the command line options (instead of an unexposed programmatic API) without executing the binary in the test suite -- including the ability to mock out the network.
Also implement
--dryrun
and--silent
.CC: @gustavnikolaj