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.

Sign up for GitHub

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

Jump to bottom

test: replace util with backtick strings #3359

Closed
wants to merge 1 commit into from
Closed

test: replace util with backtick strings #3359

wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

Now that we have backticks we no longer need to use util.format to template strings!

This commit was inspired by #3324, and it replaces instances of util.format with backtick strings in a number of tests

closes #3340 #3338 #3336

@mscdex mscdex added the test Issues and PRs related to the tests. label Oct 14, 2015
@MylesBorins
Copy link
Contributor Author

@Trott I don't know if you want to run CI again to be safe.

test: replace util with backtick strings
dafe775 
Now that we have backticks we no longer need to use util.format to template strings!

This commit was inspired by #3324, and it replaces instances of util.format with backtick strings in a number of tests
@kohei-takata
Copy link
Contributor

There seem to be two files use const at the left side of require() method, but rest two files use var.
I think all the var at the left side of require() should be const.

@MylesBorins
Copy link
Contributor Author

I only updated the files that were obvious about what was going on with variable declaration. The other tests had instances where let made more sense and figure I'd rather not start dealing with that logic for this PR. More than happy to do this.

@Trott
Copy link
Member

Trott commented Oct 14, 2015

CI: https://ci.nodejs.org/job/node-test-commit/840/

@Trott
Copy link
Member

Trott commented Oct 14, 2015

LGTM if CI doesn't do anything unexpected

@mscdex
Copy link
Contributor

mscdex commented Oct 14, 2015

LGTM

@jasnell
Copy link
Member

jasnell commented Oct 14, 2015

LGTM... will land in a sec

jasnell pushed a commit that referenced this pull request Oct 14, 2015
@jasnell
test: replace util with backtick strings
6c032a8 
Now that we have backticks we no longer need to use util.format
to template strings!

This commit was inspired by #3324, and it replaces instances of
util.format with backtick strings in a number of tests

Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3359
@jasnell
Copy link
Member

jasnell commented Oct 14, 2015

Landed in 6c032a8

... btw, @thealphanerd ... watch your line breaks in your commit messages. I fixed it up in the landed commit.

@jasnell jasnell closed this Oct 14, 2015
@MylesBorins
Copy link
Contributor Author

if tests are being backported this should be considered for lts
tag lts-watch-v4x

/cc @jasnell

@rvagg rvagg mentioned this pull request Oct 21, 2015
Merged
jasnell pushed a commit that referenced this pull request Oct 26, 2015
@jasnell
test: replace util with backtick strings
41a4258 
Now that we have backticks we no longer need to use util.format
to template strings!

This commit was inspired by #3324, and it replaces instances of
util.format with backtick strings in a number of tests

Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3359
@jasnell
Copy link
Member

jasnell commented Oct 26, 2015

Landed in v4.x-staging as 41a4258

@rvagg rvagg mentioned this pull request Oct 28, 2015
Closed
jasnell pushed a commit that referenced this pull request Oct 29, 2015
@jasnell
test: replace util with backtick strings
0a4323d 
Now that we have backticks we no longer need to use util.format
to template strings!

This commit was inspired by #3324, and it replaces instances of
util.format with backtick strings in a number of tests

Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3359
@scriptjs scriptjs mentioned this pull request Dec 2, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MylesBorins @kohei-takata @Trott @mscdex @jasnell