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: argument types for console methods #11554

Closed
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
@@ -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.)

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

@@ -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 .

@@ -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.

@@ -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
Refs: #9399
PR-URL: #11554
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
addaleax pushed a commit that referenced this pull request Mar 5, 2017
Refs: #9399
PR-URL: #11554
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@evanlucas evanlucas mentioned this pull request Mar 8, 2017
@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
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants