-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
url: change scoping of variables with let #4867
Conversation
Also changes some `var`s to `const` as they never change.
I'm confused as to what made you change IE: Line 674, |
I tried to handle all cases where a variable was redeclared. The |
LGTM if CI is OK |
@tflanagan If it's useful, here's the eslint report that @kthelgason was probably looking at to decide which instances in the file to change. https://gist.github.com/Trott/7dc16adf550b6a8d06cc |
@Trott @tflanagan yes that is indeed the report I was looking at. |
@kthelgason It looks like a problem with the CI host itself and not anything in your code. Will run CI again so we can hopefully see green on Ubuntu 15.04 and declare victory. CI again: https://ci.nodejs.org/job/node-test-pull-request/1380/ |
CI is green. LGTM |
LGTM |
Also changes some `var`s to `const` as they never change. PR-URL: nodejs#4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in fcae05e |
Also changes some `var`s to `const` as they never change. PR-URL: #4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Also changes some `var`s to `const` as they never change. PR-URL: #4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Also changes some `var`s to `const` as they never change. PR-URL: #4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Also changes some `var`s to `const` as they never change. PR-URL: #4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Also changes some `var`s to `const` as they never change. PR-URL: #4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Also changes some `var`s to `const` as they never change. PR-URL: nodejs#4867 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
This PR addresses variable redeclarations in
lib/url.js
by changing hoistedvar
s tolet
.Also changes some
var
s toconst
as recommended by the linter.@Trott points out in #4853 that the linter reports a lot of redeclarations. I believe addressing this incrementally is easier and less risky, hence this PR only fixes these issues in this one file.