Skip to content

Commit

Permalink
test: replace string concat in test-child-process-constructor
Browse files Browse the repository at this point in the history 
replace string concatenation in test/parallel/test-child-process-constructor.js
with template literals

PR-URL: #14283
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@jasnell
mac-haojin authored and jasnell committed Jul 19, 2017
1 parent 01eddd9 commit b923b9d
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions test/parallel/test-child-process-constructor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ function typeName(value) {
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options" argument must be of type object. Received type ' +
typeName(options)
message: 'The "options" argument must be of type object. ' +
`Received type ${typeName(options)}`
}));
});
}
Expand All @@ -35,8 +35,8 @@ function typeName(value) {
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.file" property must be of type string. Received ' +
'type ' + typeName(file)
message: 'The "options.file" property must be of type string. ' +
`Received type ${typeName(file)}`
}));
});
}
Expand All @@ -52,7 +52,7 @@ function typeName(value) {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.envPairs" property must be of type array. ' +
'Received type ' + typeName(envPairs)
`Received type ${typeName(envPairs)}`
}));
});
}
Expand All @@ -67,8 +67,8 @@ function typeName(value) {
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.args" property must be of type array. Received ' +
'type ' + typeName(args)
message: 'The "options.args" property must be of type array. ' +
`Received type ${typeName(args)}`
}));
});
}
Expand Down

4 comments on commit b923b9d

@trevnorris
Copy link
Contributor

@trevnorris trevnorris commented on b923b9d Jul 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@jasnell I didn't bother to add a note to this PR about changing the length of the git message header because after mentioning it in two other PRs I was told that the committer would take care of it. Please make sure the git commit message adheres to the standard (header maximum of 50 chars) with future commits.

EDIT: Also I find it troublesome that even after so many sign-offs no one caught something that is part of our default checklist when contributing a PR.

@jasnell
Copy link
Member

Choose a reason for hiding this comment

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

I did shorten it as much as practical given the long test file name.

@cjihrig
Copy link
Contributor

Choose a reason for hiding this comment

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

Also I find it troublesome that even after so many sign-offs no one caught something that is part of our default checklist when contributing a PR.

@trevnorris I know that I've personally been guilty of not paying enough attention to the commit message during the review phase. It's always something that I pay attention during landing though.

@Trott
Copy link
Member

@Trott Trott commented on b923b9d Jul 20, 2017

Choose a reason for hiding this comment

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

FWIW, I use @evanlucas's awesome core-validate-commit and always run core-validate-commit @ before landing. It's become such a natural part of my workflow and I can't recommend it enough.

In a case like this, I would remove the test name from the first line and include it in the body instead.

Please sign in to comment.