Skip to content

Conversation

earobinson
Copy link
Contributor

@earobinson earobinson commented Oct 6, 2017

test/parallel/test-string-decoder.js used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the string_decoder Issues and PRs related to the string_decoder subsystem. label Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Trott
Copy link
Member

Trott commented Oct 8, 2017

'Write sequence: ' + JSON.stringify(sequence) + '\n' +
'Full Decoder State: ' + inspect(decoder);
`Expected "${unicodeEscape(expected)}", ` +
`but got "${unicodeEscape(output)}"\n ` +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: there seems to be a trailing space here that wasn't present previously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.
@earobinson earobinson force-pushed the earobinson/test-string-decoder-template-literals branch from 0e8d4b9 to 4cd32ba Compare October 10, 2017 12:20
@lpinca
Copy link
Member

lpinca commented Oct 10, 2017

Trott pushed a commit to Trott/io.js that referenced this pull request Oct 11, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: nodejs#15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Oct 11, 2017

Landed in 3784b2d.
Thanks for the contribution! 🎉

@Trott Trott closed this Oct 11, 2017
@earobinson earobinson deleted the earobinson/test-string-decoder-template-literals branch October 11, 2017 05:16
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: nodejs/node#15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@amunro
Copy link

amunro commented Oct 12, 2017

Nice!!

targos pushed a commit that referenced this pull request Oct 18, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: #15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: #15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: #15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
`test/parallel/test-string-decoder.js` used to use string concatenation
this was migrated to use template literals. When concatenation
involved a newline we kept string concatenation, or to keep below 80
charters per line.

PR-URL: #15884
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. string_decoder Issues and PRs related to the string_decoder subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants