Skip to content
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

buffer: refactor redeclared variables #4886

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 26, 2016

A handful of variable declarations in lib/buffer.js redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to const, switching to let, or explicitly hoisting the
var declaration.

@Trott Trott added the buffer Issues and PRs related to the buffer subsystem. label Jan 26, 2016
@Trott Trott changed the title buffer: refactor redecared variables buffer: refactor redeclared variables Jan 27, 2016
@Trott
Copy link
Member Author

Trott commented Jan 27, 2016

@@ -401,10 +401,11 @@ function slowToString(encoding, start, end) {


Buffer.prototype.toString = function() {
var result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes it sense to switch the var to letto keep it in sync with line 154?

@Trott
Copy link
Member Author

Trott commented Jan 27, 2016

Green! \o/

@jasnell
Copy link
Member

jasnell commented Jan 27, 2016

lts-watch label applied.
LGTM

A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.
@Trott Trott force-pushed the buffer-no-redeclare branch from f2cf931 to 76dc775 Compare January 28, 2016 00:14
@Trott
Copy link
Member Author

Trott commented Jan 28, 2016

Small change (@romankl's suggestion) so re-running CI: https://ci.nodejs.org/job/node-test-pull-request/1416/

@r-52
Copy link
Contributor

r-52 commented Jan 28, 2016

Complete green CI - LGTM!

Trott added a commit to Trott/io.js that referenced this pull request Jan 28, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: nodejs#4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
@Trott
Copy link
Member Author

Trott commented Jan 28, 2016

Landed in 2ac47f8

@Trott Trott closed this Jan 28, 2016
rvagg pushed a commit that referenced this pull request Jan 28, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: #4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 8, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: #4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 17, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: #4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: #4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: #4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.

PR-URL: nodejs#4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
@Trott Trott deleted the buffer-no-redeclare branch January 13, 2022 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants