Skip to content

Conversation

@leizongmin
Copy link
Contributor

  • test/parallel/test-stdout-close-catch.js
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test: replace string concatenation with template literals

* test/parallel/test-stdout-close-catch.js
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jul 16, 2017
@TrottTrott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Jul 16, 2017
Copy link
Member

@TrottTrott left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green

Copy link
Member

@benjamingrbenjamingr left a comment

Choose a reason for hiding this comment

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

I'm +0 on these changes in terms of cruft ratio - but actual code looks fine.

Copy link
Member

@tniessentniessen left a comment

Choose a reason for hiding this comment

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

Same as @benjamingr.

@tniessentniessen self-assigned this Jul 16, 2017
@vsemozhetbyt
Copy link
Contributor

@Trott
Copy link
Member

CI is green! 🎉

constcmd=`${JSON.stringify(process.execPath)} `+
`${JSON.stringify(testScript)} | `+
`${JSON.stringify(process.execPath)} `+
'-pe "process.stdin.on(\'data\' , () => process.exit(1))"';

Choose a reason for hiding this comment

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

Is there any scope for improvement here? By combining this line too?

Copy link
Member

Choose a reason for hiding this comment

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

What do you suggest?

Trott pushed a commit to Trott/io.js that referenced this pull request Jul 23, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: nodejs#14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member

Landed in a5352d9.
Thanks for the contribution! 🎉

@TrottTrott closed this Jul 23, 2017
addaleax pushed a commit that referenced this pull request Jul 24, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: #14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
@addaleaxaddaleax mentioned this pull request Jul 24, 2017
@addaleaxaddaleax mentioned this pull request Aug 2, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: #14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: #14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 3, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: #14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
* test/parallel/test-stdout-close-catch.js PR-URL: #14298 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learnIssues related to the Code-and-Learn events and PRs submitted during the events.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@leizongmin@vsemozhetbyt@Trott@jasnell@benjamingr@TimothyGu@tniessen@gireeshpunathil@gibfahn@MylesBorins@nodejs-github-bot