Skip to content

Conversation

@fhalde
Copy link
Contributor

@fhalde fhalde commented May 10, 2017

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
Copy link
Contributor

@danbev danbev left a comment

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
Copy link
Contributor Author

fhalde commented May 10, 2017

Yupp

Copy link
Contributor

@mscdex mscdex May 10, 2017

Choose a reason for hiding this comment

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

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

@mscdex
Copy link
Contributor

mscdex commented May 10, 2017

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
Copy link
Contributor Author

fhalde commented May 10, 2017

done & done @mscdex

@mscdex
Copy link
Contributor

mscdex commented May 10, 2017

LGTM

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@addaleax
Copy link
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
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