lib: refactor variable declarations#22643
lib: refactor variable declarations#22643ZYSzys wants to merge 2 commits intonodejs:masterfrom zys-contrib:var2let
Conversation
nodejs-github-bot
added
the
fs
apapirovski
left a comment
apapirovski
left a comment
There was a problem hiding this comment.
Hi @ZYSzys! As you'll note by the number of places that still use var, we tend to avoid changing these for the sake of it unless the surrounding code is being refactored. Changes like this make it more difficult to use git blame efficiently.
|
I think it's more standard using |
| @@ -112,15 +112,15 @@ for (const name of Reflect.ownKeys(Dirent.prototype)) { | |||
| } | |||
|
|
|||
| function copyObject(source) { | |||
There was a problem hiding this comment.
What about Object.assign ?
There was a problem hiding this comment.
We’d probably want to benchmark that first
There was a problem hiding this comment.
It's not likely to be much slower than for-in to be honest. /cc @bmeurer
There was a problem hiding this comment.
Is this even a bit faster ?
There was a problem hiding this comment.
Recent versions of V8 should have fast Object.assign().
|
Landed in 44f7c1d. |
PR-URL: #22643 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: #22643 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
8 participants
Update
vartoconstorletChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passes