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: include the optional options parameter #7842

Closed
wants to merge 1 commit into from

Conversation

thefourtheye
Copy link
Contributor

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

doc

Description of change

mkdtemp family accepts an optional options parameter, which is
missing in the documentation.


cc @nodejs/documentation

@thefourtheye thefourtheye added the doc Issues and PRs related to the documentations. label Jul 22, 2016
@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Jul 22, 2016
@claudiorodriguez
Copy link
Contributor

In the fs docs there's already instances of this with realpath and readlink, and the description goes like this:

The optional options argument can be a string specifying an encoding, or an object with an encoding property specifying the character encoding to use for the link path passed to the callback. If the encoding is set to 'buffer', the <thing> returned will be passed as a Buffer object.

Does it make sense to go for consistency and do the same here?

@jasnell
Copy link
Member

jasnell commented Aug 5, 2016

This generally LGTM but aligning with the other instances for consistency would be good.

`mkdtemp` functions accept an optional `options` parameter, which can
be either a String specifying encoding, or an Object with an `encoding`
property.
@thefourtheye
Copy link
Contributor Author

@claudiorodriguez Updated the PR, with your suggestion. PTAL.

@jasnell
Copy link
Member

jasnell commented Aug 10, 2016

LGTM with the updates!

@jasnell
Copy link
Member

jasnell commented Aug 18, 2016

ping @claudiorodriguez ... does this LGTY now?

@claudiorodriguez
Copy link
Contributor

Sorry, LGTM

jasnell pushed a commit that referenced this pull request Aug 24, 2016
`mkdtemp` functions accept an optional `options` parameter, which can
be either a String specifying encoding, or an Object with an `encoding`
property.

PR-URL: #7842
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@jasnell
Copy link
Member

jasnell commented Aug 24, 2016

Landed in 6e50fc7

@jasnell jasnell closed this Aug 24, 2016
@Fishrock123 Fishrock123 mentioned this pull request Sep 6, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
`mkdtemp` functions accept an optional `options` parameter, which can
be either a String specifying encoding, or an Object with an `encoding`
property.

PR-URL: nodejs#7842
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
`mkdtemp` functions accept an optional `options` parameter, which can
be either a String specifying encoding, or an Object with an `encoding`
property.

PR-URL: #7842
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
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. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants