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,test: fix prime generation description #37085

Merged

Conversation

tniessen
Copy link
Member

The previous description incorrectly explained the behavior of options.add and options.rem for primes that are not safe.

@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Jan 26, 2021
@tniessen tniessen requested a review from jasnell January 26, 2021 19:52
@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 26, 2021
@nodejs-github-bot

This comment has been minimized.

@tniessen
Copy link
Member Author

@richardlau I am having trouble finding the OpenSSL version that node-test-linux-linked-openssl111fips is using once again (sorry!). Is it older than 1.1.1f?

@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 29, 2021
@nodejs-github-bot

This comment has been minimized.

@tniessen tniessen force-pushed the doc-test-fix-prime-description-add-rem branch from c21fc8d to 6b6cdcf Compare January 31, 2021 15:46
@nodejs-github-bot

This comment has been minimized.

@tniessen
Copy link
Member Author

Is it older than 1.1.1f?

Found it, it's 1.1.1c, which has a few quirks when it comes to prime number generation. Disabled the test for those versions.

PTAL assuming CI passes.

@tniessen tniessen force-pushed the doc-test-fix-prime-description-add-rem branch from 6b6cdcf to 0fdea88 Compare January 31, 2021 16:52
@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 31, 2021
The previous description incorrectly explained the behavior of
options.add and options.rem for primes that are not safe.

PR-URL: nodejs#37085
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@aduh95 aduh95 force-pushed the doc-test-fix-prime-description-add-rem branch from 0fdea88 to 814f971 Compare February 1, 2021 15:15
@aduh95 aduh95 merged commit 814f971 into nodejs:master Feb 1, 2021
@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2021

Landed in 814f971

targos pushed a commit that referenced this pull request Feb 2, 2021
The previous description incorrectly explained the behavior of
options.add and options.rem for primes that are not safe.

PR-URL: #37085
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos mentioned this pull request Feb 2, 2021
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 6, 2021
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