tools,benchmark: increase lint compliance #5429
Conversation
Trott
added
benchmark
Trott
force-pushed
the
eslint-benchmark
branch
from
4378e75
to
bca860d
Compare
Trott
changed the title
|
LGTM |
|
@nodejs/benchmarking @mscdex |
Trott
force-pushed
the
eslint-benchmark
branch
from
bca860d
to
7b5d451
Compare
In the hopes of soon having the benchmark code linted, this change groups all the likely non-controversial lint-compliance changes such as indentation, semi-colon usage, and single-vs.-double quotation marks. Other lint rules may have subtle performance implications in the V8 currently shipped with Node.js. Those changes will require more careful review and will be in a separate change.
Trott
force-pushed
the
eslint-benchmark
branch
from
7b5d451
to
261cae5
Compare
| " buff." + fn + "(0, " + JSON.stringify(noAssert) + ");", | ||
| "}" | ||
| ].join("\n")); | ||
| 'for (var i = 0; i !== ' + len + '; i++) {', |
There was a problem hiding this comment.
Couldn't we use template strings for this kind of thing now?
There was a problem hiding this comment.
@mscdex I wanted to be conservative in the change set to keep the diff manageable. And I wanted to avoid anything that might foster any discussion whatsoever about potential performance issues. If you'd prefer I go through and convert to template strings where it seems to make sense, I can do that. If left to my own devices, I'd prefer to either not do it or put that in as a separate independent pull request.
|
LGTM |
In the hopes of soon having the benchmark code linted, this change groups all the likely non-controversial lint-compliance changes such as indentation, semi-colon usage, and single-vs.-double quotation marks. Other lint rules may have subtle performance implications in the V8 currently shipped with Node.js. Those changes will require more careful review and will be in a separate change. PR-URL: nodejs#5429 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>
|
Landed in dcfda10 |
In the hopes of soon having the benchmark code linted, this change groups all the likely non-controversial lint-compliance changes such as indentation, semi-colon usage, and single-vs.-double quotation marks. Other lint rules may have subtle performance implications in the V8 currently shipped with Node.js. Those changes will require more careful review and will be in a separate change. PR-URL: #5429 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>
|
@Trott likely this should be merged in with the various benchmark changes you have been doing for a single backport PR |
|
@thealphanerd If it's all the same to you, I think it would be easier to do them as one-to-one mappings to backport PRs. This one maps to #5770 |
|
works for me... thanks! |
In the hopes of soon having the benchmark code linted, this change
groups all the likely non-controversial lint-compliance changes such as
indentation, semi-colon usage, and single-vs.-double quotation marks.
Other lint rules may have subtle performance implications in the V8
currently shipped with Node.js. Those changes will require more careful
review and will be in a separate change.