[JsConf Code & Learn]test: replace string concatenation with template literals#14342
Uh oh!
There was an error while loading. Please reload this page.
Conversation
test/parallel/test-icu-data-dir.js Outdated
| constexpected= | ||
| 'could not initialize ICU (check NODE_ICU_DATA or '+ | ||
| '--icu-data-dir parameters)'+(common.isWindows ? '\r\n' : '\n'); | ||
| `--icu-data-dir parameters)${common.isWindows ? '\r\n' : '\n'}`; |
There was a problem hiding this comment.
While you're here, could you replace ${common.isWindows ? '\r\n' : '\n'} with ${os.EOL} (you'll have to const os = require('os') after line 2 as well)?
There was a problem hiding this comment.
Actually since expected is later used with str.includes(expected) you could remove the EOL completely, and fit the string in a single line.
There was a problem hiding this comment.
@refack Sorry, I can't get your meanings. Do you mean the code afterconst expected ... to be one line?
There was a problem hiding this comment.
@refack means this:
- '--icu-data-dir parameters)' + (common.isWindows ? '\r\n' : '\n');+ '--icu-data-dir parameters)'but it was fine as it is (and arguably slightly better).
There was a problem hiding this comment.
@nathansmile sorry I missed your comment. But I landed it since it's good. 😄
refack commented Jul 18, 2017
@nathansmile thank you for the contribution. Hope you follow up on this and make it even better 👍 |
Trott added the code-and-learn 
joyeecheung commented Jul 19, 2017
@nathansmile I think @refack meant since |
joyeecheung commented Jul 19, 2017
BTW personally I think we should keep the EOL in |
joyeecheung commented Jul 19, 2017
PR-URL: nodejs#14342 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
refack commented Jul 19, 2017
Landed in 01eddd9 |
PR-URL: #14342 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: #14342 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
8 participants
replace string concatenation in test/parallel/test-icu-data-dir.js with template literals
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)