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

document how to use crypto.sign() for algorithm+digest combinations without named constants #6108

Closed
wants to merge 2 commits into from

Conversation

hillbrad
Copy link
Contributor

@hillbrad hillbrad commented Apr 7, 2016

  • documentation is changed or added
  • the commit message follows commit guidelines
Description of change

Updates the crypto API documentation to demonstrate how to use signature algorithms supported by OpenSSL but not exposed as named constants, e.g. "ecdsa-with-SHA256".


sign.update('some data to sign');

const privateKey = '-----BEGIN EC PRIVATE KEY-----\n' +

Choose a reason for hiding this comment

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

Either update this const name to be private_key for consistency, or update the one passed into sign() below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be camelcase for javascript, so the other should be fixed.

Choose a reason for hiding this comment

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

You are correct that it should be camelCase, but the rest of the references in the doc use lower_case, so, I agree that consistency with the existing examples wins.

@mscdex mscdex added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Apr 7, 2016
@hillbrad
Copy link
Contributor Author

hillbrad commented Apr 7, 2016

decided to stick with private_key for consistency with all other examples in the doc.

@jasnell
Copy link
Member

jasnell commented Apr 7, 2016

LGTM
@nodejs/crypto

@indutny
Copy link
Member

indutny commented Apr 8, 2016

LGTM

'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
'-----END EC PRIVATE KEY-----\n',
Copy link
Member

Choose a reason for hiding this comment

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

s/,/;/

jasnell pushed a commit that referenced this pull request Apr 8, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 8, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 8, 2016

Fixed the nit with the comma and landed in 9454548

@jasnell jasnell closed this Apr 8, 2016
MylesBorins pushed a commit that referenced this pull request Apr 11, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 11, 2016
MylesBorins pushed a commit that referenced this pull request Apr 19, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 20, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
This was referenced Apr 21, 2016
jasnell pushed a commit that referenced this pull request Apr 26, 2016
PR-URL: #6108
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
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.

7 participants