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: improve Sign/Verify examples and docs #25452

Closed

Conversation

sam-github
Copy link
Contributor

  • Use complete examples that can be pasted and run as-is
  • Move note about algorithm to the functions it applies to
  • Uncapitalize inconsistence use of "Class"
  • Use both EC and RSA keys in the examples
  • Note that hash and digest are two names for the same algorithms
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@sam-github sadly an error occured when I tried to trigger a build :(

@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 Jan 11, 2019
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Doc format LGTM.

doc/api/crypto.md Outdated Show resolved Hide resolved
doc/api/crypto.md Outdated Show resolved Hide resolved
doc/api/crypto.md Outdated Show resolved Hide resolved
doc/api/crypto.md Outdated Show resolved Hide resolved
algorithm, such as 'RSA-SHA256', instead of a digest algorithm. This will use
the corresponding digest algorithm. This does not work for all signature
algorithms, such as 'ecdsa-with-SHA256', so it is best to always use digest
algorithm names.
Copy link
Member

Choose a reason for hiding this comment

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

I would probably express this in an even stronger way, as far as I know, this is specific to OpenSSL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just moving the existing text closer to the API it refers to, I don't want to rework too much unless its got a serious problem. In this case, we never build with anything other than OpenSSL, so doesn't all the docs apply to OpenSSL? I'll add some specific text if you suggest, but I'm not sure what else to say, myself.

doc/api/crypto.md Outdated Show resolved Hide resolved
- Use complete examples that can be pasted and run as-is
- Move note about algorithm to the functions it applies to
- Uncapitalize inconsistence use of "Class"
- Use both EC and RSA keys in the examples
- Note that hash and digest are two names for the same algorithms
@sam-github
Copy link
Contributor Author

@sam-github
Copy link
Contributor Author

Landed in 273d7dd

@sam-github sam-github closed this Jan 14, 2019
@sam-github sam-github deleted the improve-sign-verify-docs branch January 14, 2019 23:34
sam-github added a commit that referenced this pull request Jan 14, 2019
- Use complete examples that can be pasted and run as-is
- Move note about algorithm to the functions it applies to
- Uncapitalize inconsistence use of "Class"
- Use both EC and RSA keys in the examples
- Note that hash and digest are two names for the same algorithms

PR-URL: #25452
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
- Use complete examples that can be pasted and run as-is
- Move note about algorithm to the functions it applies to
- Uncapitalize inconsistence use of "Class"
- Use both EC and RSA keys in the examples
- Note that hash and digest are two names for the same algorithms

PR-URL: #25452
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
- Use complete examples that can be pasted and run as-is
- Move note about algorithm to the functions it applies to
- Uncapitalize inconsistence use of "Class"
- Use both EC and RSA keys in the examples
- Note that hash and digest are two names for the same algorithms

PR-URL: nodejs#25452
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants