Skip to content

Conversation

@JonathanPrince
Copy link
Contributor

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

test

Description of change

change var to const/let where appropriate
use strictEqual instead of equal
call toString on buffers in strictEqual asserts
use common.mustCall on callbacks containing asserts

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 18, 2016
@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Sep 18, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to simplify we could combine this and the following assertion like assert.ok(handle !== null && typeof handle === 'object');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, that makes sense.

change var to const/let where appropriate
use strictEqual instead of equal
call toString on buffers in strictEqual asserts
use common.mustCall on callbacks containing asserts
@JonathanPrince JonathanPrince force-pushed the update_test-child-process-recv-handle branch from e1c3fea to c5b75aa Compare September 18, 2016 14:15
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

@jasnell
Copy link
Member

jasnell commented Sep 20, 2016

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Sep 20, 2016
change var to const/let where appropriate
use strictEqual instead of equal
call toString on buffers in strictEqual asserts
use common.mustCall on callbacks containing asserts

PR-URL: #8648
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@jasnell
Copy link
Member

jasnell commented Sep 20, 2016

Landed in 66dbd9c! thank you!

@jasnell jasnell closed this Sep 20, 2016
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
change var to const/let where appropriate
use strictEqual instead of equal
call toString on buffers in strictEqual asserts
use common.mustCall on callbacks containing asserts

PR-URL: #8648
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants