Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

lib: refactor crypto cipher/hash/curve getters #10682

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.

Sign up for GitHub

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

Jump to bottom
Closed
Trott wants to merge 1 commit into from
Closed

lib: refactor crypto cipher/hash/curve getters #10682

Trott wants to merge 1 commit into from

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jan 8, 2017 •
edited
Loading

  • refactor internal util.filterDuplicateStrings() to eliminate unused
    code paths
  • .indexOf() -> .includes() in test
  • more concise arrow functions
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

crypto util test lib

@Trott Trott added crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. util Issues and PRs related to the built-in util module. labels Jan 8, 2017
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. util Issues and PRs related to the built-in util module. lts-watch-v6.x labels Jan 8, 2017
@Trott
Copy link
Member Author

Trott commented Jan 8, 2017

Benchmark results:

$ node benchmark/compare.js --old ./node-master --new ./node-refactored --filter get-ciphers crypto > compare-crypto.csv
$ cat compare-crypto.csv | Rscript benchmark/compare.R
                                         improvement significant    p.value
 crypto/get-ciphers.js v="crypto" n=1         6.04 %           * 0.02953932
 crypto/get-ciphers.js v="crypto" n=5000     -0.75 %             0.66187606
 crypto/get-ciphers.js v="tls" n=1            7.92 %           * 0.02733195
 crypto/get-ciphers.js v="tls" n=5000         1.18 %             0.48262097
$

@Trott
lib: refactor crypto cipher/hash/curve getters
6c87bfc 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions
@Trott
Copy link
Member Author

Trott commented Jan 8, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/5754/

@Trott
Copy link
Member Author

Trott commented Jan 8, 2017

(CI is green.)

jasnell pushed a commit that referenced this pull request Jan 10, 2017
@Trott @jasnell
lib: refactor crypto cipher/hash/curve getters
022b53c 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: #10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 10, 2017

Landed in 022b53c

@jasnell jasnell closed this Jan 10, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@Trott
lib: refactor crypto cipher/hash/curve getters
578e542 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: nodejs#10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 23, 2017
@Trott
lib: refactor crypto cipher/hash/curve getters
10a978f 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: nodejs#10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 25, 2017
@Trott
lib: refactor crypto cipher/hash/curve getters
991f84b 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: nodejs#10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@Trott
lib: refactor crypto cipher/hash/curve getters
a6c93af 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: nodejs#10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
@Trott @MylesBorins
lib: refactor crypto cipher/hash/curve getters
4153c36 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: #10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@Trott @MylesBorins
lib: refactor crypto cipher/hash/curve getters
797d9ee 
* refactor internal util.filterDuplicateStrings() to eliminate unused
  code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions

PR-URL: #10682
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
@Trott Trott deleted the crypto-refactor branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@targos targos targos approved these changes

Assignees

No one assigned

Labels

crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott @jasnell @targos @MylesBorins @nodejs-github-bot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.