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

lib/tls: replace var with let/const by Driesss · Pull Request #30409 · nodejs/node · GitHub

/ node Public

lib/tls: replace var with let/const - #30409

Closed
Driesss wants to merge 1 commit into
nodejs:masterfrom
Driesss:nceu19
ConversationCommits (1)ChecksFiles changed
Closed

lib/tls: replace var with let/const#30409
Driesss wants to merge 1 commit into
nodejs:masterfrom
Driesss:nceu19

Conversation

Driesss commented Nov 12, 2019

Copy link
Copy Markdown
Contributor
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows [commit guidelines]

nodejs-github-bot added the tls Issues and PRs related to the tls subsystem. label Nov 12, 2019
jasnell added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 12, 2019
trivikr approved these changes Nov 13, 2019
View reviewed changes
Comment thread lib/tls.js
var offset = 0;
for (var i = 0, c = protocols.length; i < c; i++) {
let offset = 0;
for (let i = 0, c = protocols.length; i < c; i++) {

trivikr Nov 13, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nit: offset can be moved inside loop as follows:

Suggested change
for (let i = 0, c = protocols.length; i < c; i++) {
for (let offset = 0, i = 0, c = protocols.length; i < c; i++) {

nodejs-github-bot commented Nov 13, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Nov 22, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Nov 24, 2019

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Nov 24, 2019

Copy link
Copy Markdown
Collaborator

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

gireeshpunathil commented Nov 24, 2019

Copy link
Copy Markdown
Member

with couple of nasty flakes in the CI, getting a coherent result has become tough. The last two runs (26910 and 26917) together bring us a good result. I assume that should be good, and use that premise to land (at least C&L) PRs.

gireeshpunathil pushed a commit that referenced this pull request Nov 24, 2019
lib: replace var with let/const
26b2655 
PR-URL: #30409
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

gireeshpunathil commented Nov 24, 2019

Copy link
Copy Markdown
Member

landed in 26b2655, thanks for the contribution!

addaleax pushed a commit that referenced this pull request Nov 30, 2019
lib: replace var with let/const
0fd89cc 
PR-URL: #30409
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Dec 1, 2019
lib: replace var with let/const
1d6ee8b 
PR-URL: #30409
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BridgeAR mentioned this pull request Dec 3, 2019
Merged
BethGriggs mentioned this pull request Dec 9, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
lib: replace var with let/const
03e6d0d 
PR-URL: #30409
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BethGriggs mentioned this pull request Dec 23, 2019
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
gireeshpunathil gireeshpunathil approved these changes
BridgeAR BridgeAR approved these changes
trivikr trivikr approved these changes

Assignees

No one assigned

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL