Skip to content
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

doc: Rectify error in repl defineCommand tutorial #7365

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

akki
Copy link
Contributor

@akki akki commented Jun 22, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc, repl

Description of change

Fixes: #7357

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Jun 22, 2016
@akki akki force-pushed the repl-doc branch 3 times, most recently from dc7bb4a to 87452ab Compare June 22, 2016 11:40
@cjihrig
Copy link
Contributor

cjihrig commented Jun 22, 2016

LGTM

2 similar comments
@mscdex
Copy link
Contributor

mscdex commented Jun 22, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Jun 22, 2016

LGTM

@@ -312,12 +312,12 @@ replServer.defineCommand('sayhello', {
action: function(name) {
this.lineParser.reset();
this.bufferedCommand = '';
this.write(`Hello, ${name}!\n`);
console.log(`Hello, ${name}!\n`);
Copy link
Contributor

Choose a reason for hiding this comment

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

both should not have the '\n' when using console.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated PR accordingly.

@akki
Copy link
Contributor Author

akki commented Jun 23, 2016

Please let me know if there's anything else I can do to help getting this issue closed. Thanks.

@cjihrig cjihrig merged commit f3114e2 into nodejs:master Jun 23, 2016
@addaleax
Copy link
Member

@cjihrig Did you merge this with the github merge button? The Author: field is different in the landed commit from the one in this PR.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 23, 2016

Yes, I did.

@addaleax
Copy link
Member

okay, good to know that that can happen

Fishrock123 pushed a commit that referenced this pull request Jun 27, 2016
Fixes: #7357
PR-URL: #7365
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Jun 27, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
Fixes: #7357
PR-URL: #7365
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replServer defineCommand
8 participants