Skip to content

doc: Update doc of publicEncrypt method#12947

Closed
fhalde wants to merge 1 commit into
nodejs:masterfrom
fhalde:doc
Closed

doc: Update doc of publicEncrypt method#12947
fhalde wants to merge 1 commit into
nodejs:masterfrom
fhalde:doc

Conversation

@fhalde

@fhalde fhalde commented May 10, 2017

Copy link
Copy Markdown
Contributor

As per #12946
the crypto doc for publicEncrypt doesn't tell
you whether the encryption happens in place or not.

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels May 10, 2017

@danbev danbev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: could you limit the line length to 80 characters so that is consistent?

@fhalde

fhalde commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

Yupp

Comment thread doc/api/crypto.md Outdated

@mscdex mscdex May 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/buffer/[`Buffer`][]/

@mscdex

mscdex commented May 10, 2017

Copy link
Copy Markdown
Contributor

While you're in there, would you mind changing private_key to public_key in both of the public*() method parameter descriptions?

As per nodejs#12946
the crypto doc for publicEncrypt doesn't tell
you whether the encryption happens in place or not.
@fhalde

fhalde commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

done & done @mscdex

@mscdex

mscdex commented May 10, 2017

Copy link
Copy Markdown
Contributor

LGTM

@mhdawson mhdawson left a comment

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.

LGTM

@addaleax

Copy link
Copy Markdown
Member

Landed in eff9252

@addaleax addaleax closed this May 19, 2017
addaleax pushed a commit that referenced this pull request May 19, 2017
As per #12946
the crypto doc for publicEncrypt doesn't tell
you whether the encryption happens in place or not.

Fixes: #12946
PR-URL: #12947
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
As per nodejs#12946
the crypto doc for publicEncrypt doesn't tell
you whether the encryption happens in place or not.

Fixes: nodejs#12946
PR-URL: nodejs#12947
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@MylesBorins

Copy link
Copy Markdown
Contributor

v6.x?

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

Labels

crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants