Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jan 29, 2016

Two variables are declared twice with var in the same scope in
lib/fs.js. This change refactors the code so the variable is declared
just once.

Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.
@Trott Trott added fs Issues and PRs related to the fs subsystem / file system. lts-watch-v4.x labels Jan 29, 2016
@bnoordhuis
Copy link
Member

LGTM

@Trott
Copy link
Member Author

Trott commented Jan 30, 2016

@mscdex
Copy link
Contributor

mscdex commented Jan 30, 2016

LGTM

lib/fs.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

this is fine but...

const nextPartRe = isWindows ? 
  /(.*?)(?:[\/\\]+|$)/g : 
  /(.*?)(?:[\/]+|$)/g;

const splitRootRe = isWindows ?
  /^(?:[a-zA-Z]:|[\\\/]{2}[^\\\/]+[\\\/][^\\\/]+)?[\\\/]*/ :
  /^[\/]*/;

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about doing that, but wanted to keep the change as small as reasonable. But that was a guideline I applied and not a hard rule, so yeah, I'm happy to make that change.

@jasnell
Copy link
Member

jasnell commented Feb 1, 2016

minor suggestion but LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Feb 1, 2016

LGTM with @jasnell's comment.

@Trott
Copy link
Member Author

Trott commented Feb 1, 2016

Switched to ternary operator per comment from @jasnell.

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

Trott added a commit to Trott/io.js that referenced this pull request Feb 1, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: nodejs#4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@Trott
Copy link
Member Author

Trott commented Feb 1, 2016

Landed in 7a2a551

@Trott Trott closed this Feb 1, 2016
rvagg pushed a commit that referenced this pull request Feb 8, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: #4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 17, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: #4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: #4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: #4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.

PR-URL: nodejs#4959
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@Trott Trott deleted the fs-no-redeclare branch January 13, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants