FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: Remove XXX comment in readline and change to output debuglog by kohei-takata · Pull Request #4690 · nodejs/node · GitHub

/ node Public

lib: Remove XXX comment in readline and change to output debuglog - #4690

Closed
kohei-takata wants to merge 1 commit into
nodejs:masterfrom
kohei-takata:remove_xxx_in_readline
ConversationCommits (1)ChecksFiles changed
Closed

lib: Remove XXX comment in readline and change to output debuglog#4690
kohei-takata wants to merge 1 commit into
nodejs:masterfrom
kohei-takata:remove_xxx_in_readline

Conversation

kohei-takata commented Jan 14, 2016

Copy link
Copy Markdown
Contributor

refs #4642
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

yosuke-furukawa reviewed Jan 14, 2016
View reviewed changes
Comment thread lib/readline.js Outdated

yosuke-furukawa Jan 14, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Congratulations your 1st contribution.

You would be better to print more information. e.g.

debug('tab complete error ', err);

like https://github.com/nodejs/node/blob/master/lib/repl.js#L230

yosuke-furukawa added the good first issue Issues that are suitable for first-time contributors. label Jan 14, 2016
Fishrock123 added readline Issues and PRs related to the built-in readline module. and removed good first issue Issues that are suitable for first-time contributors. labels Jan 14, 2016

Fishrock123 commented Jan 14, 2016

Copy link
Copy Markdown
Contributor

@kohei-takata Could you please prefix the commit message with readline: instead of lib:?

Also please make sure the commit message (the first line, where the prefix is) is no longer than 50 characters. :)

jasnell commented Jan 14, 2016

Copy link
Copy Markdown
Member

LGTM with the nits @Fishrock123 and @yosuke-furukawa mentioned addressed.

readline: Remove XXX and output debuglog
a854ec6 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.
kohei-takata force-pushed the remove_xxx_in_readline branch from d506391 to a854ec6 Compare January 15, 2016 03:51

kohei-takata commented Jan 15, 2016

Copy link
Copy Markdown
Contributor Author

@yosuke-furukawa
Thanks 😄
I've changed arguments of debug().

@Fishrock123
Thanks for your review!
I've changed prefix of the commit message and shorten 😄

@jasnell
Thanks!

cjihrig commented Jan 15, 2016

Copy link
Copy Markdown
Contributor

LGTM pending CI

Fishrock123 commented Jan 15, 2016

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/1255/

jasnell commented Jan 15, 2016

Copy link
Copy Markdown
Member

Failures in CI look unrelated. Updated commit LGTM

Trott commented Jan 16, 2016

Copy link
Copy Markdown
Member

CI is a little more robust right now then it was 13 hours ago, so let's try CI again:
https://ci.nodejs.org/job/node-test-pull-request/1268/

Trott commented Jan 16, 2016

Copy link
Copy Markdown
Member

CI is green! \o/

jasnell commented Jan 16, 2016

Copy link
Copy Markdown
Member

Great job @Trott. Great to see.
On Jan 15, 2016 9:24 PM, "Rich Trott" notifications@github.com wrote:

CI is green! \o/

—
Reply to this email directly or view it on GitHub
#4690 (comment).

kohei-takata commented Jan 16, 2016

Copy link
Copy Markdown
Contributor Author

@Trott Thanks!

jasnell pushed a commit that referenced this pull request Jan 18, 2016
readline: Remove XXX and output debuglog
ad63d35 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: #4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Jan 18, 2016

Copy link
Copy Markdown
Member

Landed in ad63d35
Thank you @kohei-takata !

jasnell closed this Jan 18, 2016
evanlucas mentioned this pull request Jan 18, 2016
Merged
evanlucas pushed a commit that referenced this pull request Jan 18, 2016
readline: Remove XXX and output debuglog
d533364 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: #4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
readline: Remove XXX and output debuglog
cb72043 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: #4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
readline: Remove XXX and output debuglog
617faf7 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: #4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
readline: Remove XXX and output debuglog
c2b3963 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: nodejs#4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Feb 11, 2016
Merged
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 13, 2016
readline: Remove XXX and output debuglog
d54d92b 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: nodejs#4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
readline: Remove XXX and output debuglog
4fbcb47 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: nodejs#4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
readline: Remove XXX and output debuglog
2b90c3a 
Remove a comment that has a word 'XXX'.
And add a line to output debuglog of error.

PR-URL: nodejs#4690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

readline Issues and PRs related to the built-in readline module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL