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
fs: update fs module #10591
fs: update fs module #10591
Conversation
- replace var with const and let
I'm not sure we're converting core modules (wholesale) yet, since there may still be some performance concerns at the very least ... |
return m; | ||
if (typeof m === 'string') | ||
} else if (typeof m === 'string') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These may not be necessary, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm -1 on this PR. I think it will degrade blame
without adding much improvement and it also requires a lot of work to properly review.
sorry for the misunderstand, I thought it would a cool idea to make these change in the core modules (fs) anyway here is another commit where I change cc: @Trott |
Lots of conflicts. Can you rebase? |
- replace var with const and let
The commit that makes changes to |
the goal of this PR is to use local variable by replacing
var
withconst
andlet
.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
fs