FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: refactor crypto cipher/hash/curve getters by Trott · Pull Request #10682 · nodejs/node · GitHub

/ node Public

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 commented Jan 8, 2017
edited
Loading

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

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 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 commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

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 commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

Trott commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

(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 commented Jan 10, 2017

Copy link
Copy Markdown
Member

Landed in 022b53c

jasnell 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>
italoacasas 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>
MylesBorins mentioned this pull request Mar 9, 2017
Trott deleted the crypto-refactor branch January 13, 2022 22:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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. 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

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL