Skip to content

removes readline module, see #49, general clean ocd, added `options.clea... - #58

Merged
hallas merged 1 commit into
masterfrom
no-readline
Mar 25, 2014
Merged

hallas merged 1 commit into
masterfrom
no-readline

Conversation

@hallas

@hallas hallas commented Mar 21, 2014

Copy link
Copy Markdown
Contributor

hi @jdan take a look at this, -22 net line changes and readline is gone :-)

Comment thread lib/node-progress.js Outdated

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it do?

Copy link
Copy Markdown
Collaborator

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

@jdan

jdan commented Mar 21, 2014

Copy link
Copy Markdown
Collaborator

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?

@hallas

hallas commented Mar 21, 2014

Copy link
Copy Markdown
Contributor Author

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 :)

@jdan

jdan commented Mar 21, 2014

Copy link
Copy Markdown
Collaborator

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 :)

@hallas

hallas commented Mar 21, 2014

Copy link
Copy Markdown
Contributor Author

how would you pipe it to a file? I'm curious (i did not test this)

@jdan

jdan commented Mar 21, 2014

Copy link
Copy Markdown
Collaborator

$ node examples/blah.js 2> /dev/null—
Sent from Mailbox for iPhone

On Fri, Mar 21, 2014 at 10:48 AM, Christoffer Hallas
notifications@github.com wrote:

how would you pipe it to a file? I'm curious (i did not test this)

Reply to this email directly or view it on GitHub:
#58 (comment)

@hallas

hallas commented Mar 21, 2014

Copy link
Copy Markdown
Contributor Author

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 ;)

@jdan

jdan commented Mar 21, 2014

Copy link
Copy Markdown
Collaborator

Sweet :) Feel free to merge and close #49 if all looks good—
Sent from Mailbox for iPhone

On Fri, Mar 21, 2014 at 10:52 AM, Christoffer Hallas
notifications@github.com wrote:

Oh right I thought you wanted to pipe the actual final "progress bar" to a file :) it works then

Reply to this email directly or view it on GitHub:
#58 (comment)

@hallas

hallas commented Mar 21, 2014

Copy link
Copy Markdown
Contributor Author

I'll give TJ a chance to intervene if he feels like it, no need to rush it

@hallas

hallas commented Mar 24, 2014

Copy link
Copy Markdown
Contributor Author

@jdan would you mind doing a thorough test? then I'll merge in and publish it

@hallas

hallas commented Mar 25, 2014

Copy link
Copy Markdown
Contributor Author

@jdan I added missing change logs to history.md

Comment thread lib/node-progress.js Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this as expose :)

@hallas

hallas commented Mar 25, 2014

Copy link
Copy Markdown
Contributor Author

otherwise we're good : )?

@jdan

jdan commented Mar 25, 2014

Copy link
Copy Markdown
Collaborator

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
hallas pushed a commit that referenced this pull request Mar 25, 2014
bump to version 1.1.5, added missing changelogs, make now runs all examples, removed readline dependency and a little ocd
@hallas
hallas merged commit 66495c5 into master Mar 25, 2014
@hallas

hallas commented Mar 25, 2014

Copy link
Copy Markdown
Contributor Author

closing, fixes #49

@hallas
hallas deleted the no-readline branch March 25, 2014 16:17
@hallas hallas mentioned this pull request Mar 25, 2014
@jdan

jdan commented Mar 25, 2014

Copy link
Copy Markdown
Collaborator

👏

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.

2 participants