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: fix return type #29696

Closed
wants to merge 2 commits into from
Closed

doc: fix return type #29696

wants to merge 2 commits into from

Conversation

exoego
Copy link
Contributor

@exoego exoego commented Sep 24, 2019

IIUC, crypto.createDiffieHellmanGroup(name) and its original crypto.getDiffieHellman(groupName) will return a instance of DiffieHellmanGroup which is not a sub type of DiffieHellman.

const instnace = crypto.createDiffieHellmanGroup("modp14")

console.log(instnace instanceof crypto.DiffieHellmanGroup)
// true
console.log(instnace instanceof crypto.DiffieHellman)
// false

on Node.js v12.10.0.

Checklist

@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 Sep 24, 2019
@Trott
Copy link
Member

Trott commented Sep 25, 2019

@nodejs/crypto

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Sep 25, 2019

If this will be accepted, we also need to update the type parser, i.e. to add an entry after this line:

'DiffieHellman': 'crypto.html#crypto_class_diffiehellman',

@Trott Trott added the review wanted PRs that need reviews. label Sep 27, 2019
@Trott
Copy link
Member

Trott commented Sep 27, 2019

If this will be accepted, we also need to update the type parser, i.e. to add an entry after this line:

'DiffieHellman': 'crypto.html#crypto_class_diffiehellman',

It will have to happen in this PR or else make doc will fail, I think?

@exoego
Copy link
Contributor Author

exoego commented Sep 28, 2019

If this will be accepted, we also need to update the type parser, i.e. to add an entry after this line:

'DiffieHellman': 'crypto.html#crypto_class_diffiehellman',

It will have to happen in this PR or else make doc will fail, I think?

Added in 87b26aa

@Trott
Copy link
Member

Trott commented Sep 28, 2019

@nodejs/crypto @nodejs/documentation This could use reviews.

@Trott
Copy link
Member

Trott commented Oct 1, 2019

@nodejs/collaborators

@Trott
Copy link
Member

Trott commented Oct 4, 2019

Rebased and force-pushed.

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3970/

Trott pushed a commit that referenced this pull request Oct 4, 2019
@Trott
Copy link
Member

Trott commented Oct 4, 2019

Landed in 03b7a5a

@Trott Trott closed this Oct 4, 2019
@exoego exoego deleted the doc-crypto branch October 4, 2019 02:53
BridgeAR pushed a commit that referenced this pull request Oct 9, 2019
@BridgeAR BridgeAR mentioned this pull request Oct 10, 2019
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. review wanted PRs that need reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants