console: using class#9068
console: using class#9068Naltox wants to merge 4 commits intonodejs:masterfrom Naltox:console-using-class
Conversation
nodejs-github-bot
added
the
console
addaleax
added
the
semver-major
lib/console.js
Outdated
| class Console { | ||
| constructor(stdout, stderr) { | ||
| if (!(this instanceof Console)) | ||
| return new Console(stdout, stderr); |
There was a problem hiding this comment.
This part is not going to work, so you can leave it out from this PR.
| // As of v8 5.0.71.32, the combination of rest param, template string | ||
| // and .apply(null, args) benchmarks consistently faster than using | ||
| // the spread operator when calling util.format. | ||
| log(...args) { |
There was a problem hiding this comment.
I believe this is still slow, even with V8 5.4 currently in master.
Ditto for the other rest args uses.
|
Have you performed benchmarks that tests these changes? |
|
This change will likely be difficult to backport, just changing it is of little value.... was anything else improved? Can it be improved without using |
|
-1 on this for the moment. Large rewrites like this across the code base are prone to error and make debugging harder as it will completely wash the blame on the document. There are also potential performance concerns as mentioned by @mscdex further, since it is semver major, this is going to make any backporting unnecessarily difficult What are the reasons for the change aside from adopting new features |
6 participants
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesDescription of change
Just rewritten Console using ES6 class.