Uh oh!
There was an error while loading. Please reload this page.
tools,benchmark: increase lint compliance - #5429
Conversation
silverwind
commented
Feb 25, 2016
LGTM |
Trott
commented
Feb 27, 2016
@nodejs/benchmarking @mscdex |
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
commented
Feb 27, 2016
| " 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.
mscdex
commented
Feb 28, 2016
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>
Trott
commented
Feb 28, 2016
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>
MylesBorins
commented
Mar 17, 2016
@Trott likely this should be merged in with the various benchmark changes you have been doing for a single backport PR |
Trott
commented
Mar 17, 2016
@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 |
MylesBorins
commented
Mar 17, 2016
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.