Skip to content

Conversation

@kevinwcyu
Copy link
Contributor

Replace string concatenation in tools/doc/preprocess.js with template literals.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)
  • tools
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Nov 6, 2017
@gireeshpunathil gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 6, 2017
Copy link
Member

Choose a reason for hiding this comment

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

lost indentation here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Multi-line string concatenation is actually ok (the way it was before), but it would be nice if the 2nd line became this instead:

`${inc}\n<!-- [end-include:${fname}] -->\n`

That way we don't have to lose indentation level.

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 6, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: nodejs#16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@Trott
Copy link
Member

Trott commented Nov 6, 2017

Landed in 13e983b

@Trott Trott closed this Nov 6, 2017
@Trott
Copy link
Member

Trott commented Nov 6, 2017

Thanks for the contribution! 🎉

cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 7, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: nodejs#16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@cjihrig cjihrig mentioned this pull request Nov 7, 2017
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: #16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: #16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
@gibfahn gibfahn mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: #16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
Replace string concatenation with template literals in tools/doc/preprocess.js. PR-URL: #16804 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.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. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

7 participants