util: remove duplicate code in format #15098
Conversation
util.format contains an idential if statement within each branch of switch. Move it before the switch statement for cleaner code and better performance.
nodejs-github-bot
added
the
util
|
I think this does not need the typical 48 hours. |
|
@BridgeAR generally, only doc updates can get a pass for the 48 rule... |
|
@evanlucas oh, alright. I somehow remembered it would also be trivial fixes and the code change seems pretty simple to me and there were already a lot of looking goods. That is why I suggested it. Thanks for the info! |
|
Hm, even though the docs say otherwise. I cite https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#accepting-modifications
|
|
@BridgeAR ah fair point. My bad :] |
|
Landed in 365c245 after asking @evanlucas about his OK again. |
util.format contains an idential if statement within each branch of switch. Move it before the switch statement for cleaner code and better performance. PR-URL: #15098 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
util.format contains an idential if statement within each branch of switch. Move it before the switch statement for cleaner code and better performance. PR-URL: nodejs/node#15098 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
util.format contains an idential if statement within each branch of switch. Move it before the switch statement for cleaner code and better performance. PR-URL: #15098 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
util.format contains an idential if statement within each branch of switch. Move it before the switch statement for cleaner code and better performance. PR-URL: #15098 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
|
Should this be backported to |
|
@MylesBorins it could be (easily) since this was quite a minor change but there are commits between where this landed and where 6.x is at. Not sure what the general policy is around back-porting. Specifically: 4191962 and a46c43d I'm guessing the latter might be out of scope for backporting since it introduces new functionality but the former is just a performance change. |
util.formatcontained an identical if statement within each case of switch. Moved it before the switch statement for cleaner code and better performance. Passes all the current tests, plus here are the benchmark results (20 sets on old, 20 sets on new):Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
util