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.

Sign up for GitHub

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

Jump to bottom

doc: argument types for console methods #11554

Closed
wants to merge 1 commit into from
Closed

doc: argument types for console methods #11554

wants to merge 1 commit into from

Conversation

ameliavoncat
Copy link
Contributor

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

documentation

Description of changes

Added argument data types to the docs for the console module.

Issue

@nodejs-github-bot nodejs-github-bot added console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. labels Feb 25, 2017
joyeecheung
joyeecheung reviewed Feb 25, 2017
View reviewed changes
doc/api/console.md Outdated
@@ -73,6 +73,8 @@ const Console = console.Console;
```

### new Console(stdout[, stderr])
* ```stdout``` {Writable Stream}
Copy link
Member

Choose a reason for hiding this comment

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

The actual class should be Writable, which is also used by documentations of repl and readline.

(Also, why triple backticks? Not that it really matters though, just out of curiosity.)

doc/api/console.md Outdated
@@ -162,6 +167,8 @@ console.log('this will also print');
<!-- YAML
added: v0.1.101
-->
* ```obj``` {any}
* ```options``` {Object}
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, annotations for the available options, like what the documentations of child_process. execFile() does would be nice (Doesn't need to happen in this PR though)

doc/api/console.md Outdated
@@ -101,6 +103,9 @@ new Console(process.stdout, process.stderr);
<!-- YAML
added: v0.1.101
-->
* ```value``` {Boolean}
* ```message``` {String}
* ```...args``` {String}
Copy link
Member

Choose a reason for hiding this comment

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

...args doesn't really have a type here, they can be anything of any length .

doc/api/console.md Outdated
@@ -271,6 +286,8 @@ functionality was unintended and is no longer supported.*
<!-- YAML
added: v0.1.104
-->
* ```message``` {String}
* ```...args``` {String}
Copy link
Member

Choose a reason for hiding this comment

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

...args here doesn't have a type either.

@ameliavoncat
Copy link
Contributor Author

Changes made.

lpinca
lpinca reviewed Feb 25, 2017
View reviewed changes
doc/api/console.md Outdated
@@ -271,6 +289,8 @@ functionality was unintended and is no longer supported.*
<!-- YAML
added: v0.1.104
-->
* `message` {String}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: maybe it's better to use any.

Copy link
Member

Choose a reason for hiding this comment

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

Actually… message is optional too, it would be good if you could fix that in the ### console.trace(…) line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@addaleax
Copy link
Member

addaleax commented Mar 1, 2017

Landed in 87a039d, thanks for the awesome PR!

@addaleax addaleax closed this Mar 1, 2017
addaleax pushed a commit that referenced this pull request Mar 1, 2017
@ameliavoncat @addaleax
doc: argument types for console methods
87a039d 
Refs: #9399
PR-URL: #11554
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
addaleax pushed a commit that referenced this pull request Mar 5, 2017
@ameliavoncat @addaleax
doc: argument types for console methods
e468cd3 
Refs: #9399
PR-URL: #11554
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@evanlucas evanlucas mentioned this pull request Mar 8, 2017
Merged
@MylesBorins
Copy link
Contributor

It looks like some of these interfaces might be different on v6.x. Would someone be willing to backport?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

Assignees
No one assigned
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ameliavoncat @addaleax @MylesBorins @lpinca @cjihrig @joyeecheung @nodejs-github-bot