-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
benchmark: use strict mode #5336
Conversation
Apply strict mode to benchmark code.
@trevnorris @bnoordhuis ... any insight into how this would affect the benchmarks? |
LGTM, I take it this is preparation to lint the benchmarks? |
LGTM
Strict mode should not have a negative impact on the benchmark numbers, if that is what you're asking. It might even improve them. |
Yep, that's what I was asking! I'm not overly familiar with the internals
|
@silverwind asks:
That's the hope. |
LGTM |
1 similar comment
LGTM |
@@ -1,3 +1,4 @@ | |||
'use strict'; |
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.
maybe add a space on the 2nd line for these?
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.
In our current tests, it is atypical to have a blank line after use strict
. (In test/parallel
, there are 943 tests that contain use strict
. Of those, only 108 contain a blank line after use strict
.) So I favored the prevailing convention of no blank line. (Although I see I was not quite 100% consistent with that here. Ugh.)
Anyway, I don't have any strong feelings about it, but that's why I did it the way I did.
Apply strict mode to benchmark code. PR-URL: nodejs#5336 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Landed in 4bb529d. It's worth mentioning perhaps that
|
Apply strict mode to benchmark code. PR-URL: #5336 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Trott would you be willing to backport this to v4.x? |
@thealphanerd If you can get #5123 backported first, that will make this one go a little easier. |
@Trott the path improvements are not likely to land soon... if ever |
OK, I'll backport this along with the others then. |
Apply strict mode to benchmark code.