Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: use template literals #9120

Closed
wants to merge 1 commit into from
Closed

util: use template literals #9120

wants to merge 1 commit into from

Conversation

a0viedo
Copy link
Member

@a0viedo a0viedo commented Oct 16, 2016

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

Util

Description of change

This commit changes string manipulation in favor of template literals in the util module.

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Oct 16, 2016
@mscdex
Copy link
Contributor

mscdex commented Oct 16, 2016

@a0viedo
Copy link
Member Author

a0viedo commented Oct 16, 2016

oh the linter was failing for a long line, gonna make that fix once CI finishes

@evanlucas
Copy link
Contributor

I'd be curious to see if this causes any performance regressions in util.format/util.inspect

@targos
Copy link
Member

targos commented Oct 17, 2016

There should be no change in performance. AFAIK untagged template literals are desugared to string concatenation by V8's parser.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

This commit changes string manipulation in favor of template literals in the `util` module.
@a0viedo
Copy link
Member Author

a0viedo commented Oct 17, 2016

Fixed linter.

@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@jasnell
Copy link
Member

jasnell commented Oct 19, 2016

@jasnell
Copy link
Member

jasnell commented Oct 24, 2016

New CI since the last one was aborted: https://ci.nodejs.org/job/node-test-pull-request/4637/

jasnell pushed a commit that referenced this pull request Oct 26, 2016
This commit changes string manipulation in favor of template
literals in the `util` module.

PR-URL: #9120
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 26, 2016

Landed in 1ee36f6

@jasnell jasnell closed this Oct 26, 2016
evanlucas pushed a commit that referenced this pull request Nov 3, 2016
This commit changes string manipulation in favor of template
literals in the `util` module.

PR-URL: #9120
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
This commit changes string manipulation in favor of template
literals in the `util` module.

PR-URL: #9120
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
This commit changes string manipulation in favor of template
literals in the `util` module.

PR-URL: #9120
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This was referenced Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants