-
Notifications
You must be signed in to change notification settings - Fork 222
removes readline module, see #49, general clean ocd, added `options.clea... #58
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
Conversation
lib/node-progress.js
Outdated
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.
Not a great idea to remove this
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.
what does it do?
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.
It's just license info
|
I like it, I'm a little hesitant to merge stylistic changes on a project I haven't started - but it's your call. Let @visionmedia chime in? |
|
yeah I understand, just trying to keep the code style moving with "todays" ways. I know it's very opinionated though. I'm up for whatever :) |
|
Okay just make sure it works when you pipe to a file and stuff like that (looks like it should). I'm cool with this being merged, thanks :) |
|
how would you pipe it to a file? I'm curious (i did not test this) |
|
$ node examples/blah.js 2> /dev/null— On Fri, Mar 21, 2014 at 10:48 AM, Christoffer Hallas
|
|
Oh right I thought you wanted to pipe the actual final "progress bar" to a file :) it still outputs (along as you don't set the stream to stdout) when piping ;) |
|
Sweet :) Feel free to merge and close #49 if all looks good— On Fri, Mar 21, 2014 at 10:52 AM, Christoffer Hallas
|
|
I'll give TJ a chance to intervene if he feels like it, no need to rush it |
|
@jdan would you mind doing a thorough test? then I'll merge in and publish it |
|
@jdan I added missing change logs to history.md |
lib/node-progress.js
Outdated
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.
Keep this as expose :)
|
otherwise we're good : )? |
|
I would put the license info back - but otherwise you're free to merge it. I won't nitpick |
…amples, removed readline dependency and a little ocd
bump to version 1.1.5, added missing changelogs, make now runs all examples, removed readline dependency and a little ocd
|
closing, fixes #49 |
|
👏 |
hi @jdan take a look at this, -22 net line changes and readline is gone :-)