Conversation
2bf76ed to
3a01e41
Compare
lib/_http_outgoing.js
Outdated
There was a problem hiding this comment.
I change var to let in my first try. But I got
/home/travis/build/nodejs/node/lib/_http_outgoing.js
802:8 error Use of `let` as the loop variable in a for-loop is not recommended. Please use `var` instead node-core/no-let-in-for-declarationCan we remove this rule in master branch ?
cc @BridgeAR @Trott
There was a problem hiding this comment.
It has been two years since #8637 (when using let/const lost performance), and let/const performance in V8 was improved due to #8637 (comment) and #9729 (comment)
I think maybe it's time to remove this rule now ?
There was a problem hiding this comment.
That rule was instituted because using let with a for loop that way could be drastically slower than using var. The problem was fixed in V8 6.0.
This performance problem still exists in Node.js 6.x (which ships with V8 5.1). Since code that lands on the master branch can be backported to 6.x, the rule remains in place. Support for 6.x ends this June, so starting in July, we can disable the rule. At that time, the oldest supported Node.js version will be 8.x, which currently ships with V8 6.2.
There was a problem hiding this comment.
Or start a pr and add the label don't land on 6.x ?
There was a problem hiding this comment.
6.x is in maintenance. I think we can already disable the rule.
3a01e41 to
d768c5c
Compare
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #26562 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes