Skip to content

Conversation

@wzoom
Copy link
Contributor

@wzoom wzoom commented Sep 17, 2016

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

test

Description of change

Changed var --> const and let.
Changed assert.notEqual --> assert.notStrictEqual
Fixed comment spelling
Took as part of nodejs/code-and-learn#56

Changed var --> const and let.
Changed assert.notEqual --> assert.notStrictEqual
Fixed comment spelling
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2016
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM

@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Sep 17, 2016
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM with a green CI run

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.

Copy link
Contributor

@MylesBorins MylesBorins 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

@wzoom
Copy link
Contributor Author

wzoom commented Sep 19, 2016

Could you run the tests again? There is 1 failed test, that is not tied to these changes AFAIK. Check it please. Or tell me what is needed in such case, because I don't know.

@jasnell
Copy link
Member

jasnell commented Sep 20, 2016

The failure is unrelated. There shouldn't be a need to run it again :-)

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 pushed a commit that referenced this pull request Sep 20, 2016
Changed var --> const and let.
Changed assert.notEqual --> assert.notStrictEqual
Fixed comment spelling

PR-URL: #8618
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 20, 2016

Landed in 3ffa6a8! thank you!

@jasnell jasnell closed this Sep 20, 2016
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Changed var --> const and let.
Changed assert.notEqual --> assert.notStrictEqual
Fixed comment spelling

PR-URL: #8618
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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.

8 participants