Skip to content

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

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:crypto-refactor
Closed

lib: refactor crypto cipher/hash/curve getters#10682
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:crypto-refactor

Conversation

@Trott

@TrottTrott commented Jan 8, 2017

Copy link
Copy Markdown
Member
  • 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

@TrottTrott 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-botnodejs-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

Trott commented Jan 8, 2017

Copy link
Copy Markdown
MemberAuthor

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
$ 

* refactor internal util.filterDuplicateStrings() to eliminate unused
code paths
* `.indexOf()` -> `.includes()` in test
* more concise arrow functions
@Trott

Trott commented Jan 8, 2017

Copy link
Copy Markdown
MemberAuthor

@Trott

Trott commented Jan 8, 2017

Copy link
Copy Markdown
MemberAuthor

(CI is green.)

jasnell pushed a commit that referenced this pull request Jan 10, 2017
* 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
Copy Markdown
Member

Landed in 022b53c

@jasnelljasnell closed this Jan 10, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
* 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
* 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
* 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 27, 2017
* 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>
@italoacasasitaloacasas mentioned this pull request Jan 29, 2017
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
* 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
* 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>
@MylesBorinsMylesBorins mentioned this pull request Mar 9, 2017
@Trott
Trott deleted the crypto-refactor branch January 13, 2022 22:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cryptoIssues and PRs related to the crypto subsystem.lib / srcIssues and PRs related to general changes in the lib or src directory.testIssues and PRs related to the tests.utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott@jasnell@targos@MylesBorins@nodejs-github-bot