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 assert.equal -> assert.strictEqual
Cleanup according to nodejs/code-and-learn#56

Changed var -> const and assert.equal -> assert.strictEqual
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Sep 17, 2016

LGTM with a comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you line up the first single quote with the one on the line above it.

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 with @cjihrig's comment

Wrapped line longer than 80 chars to more lines.
@wzoom
Copy link
Contributor Author

wzoom commented Sep 17, 2016

Fixed.

@targos
Copy link
Member

targos commented Sep 17, 2016

@wzoom
Copy link
Contributor Author

wzoom commented Sep 19, 2016

Weird CI failure, actually the same as in #8618 Can you try to re-run the tests?

@lpinca
Copy link
Member

lpinca commented Sep 19, 2016

@wzoom the only failure (test/arm-fanned) is known to be flaky and is not related to this change.

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

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 22, 2016
Changed var -> const and assert.equal -> assert.strictEqual

PR-URL: #8590
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 22, 2016

Landed in 070efb5. Thank you!

@jasnell jasnell closed this Sep 22, 2016
jasnell pushed a commit that referenced this pull request Sep 29, 2016
Changed var -> const and assert.equal -> assert.strictEqual

PR-URL: #8590
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Changed var -> const and assert.equal -> assert.strictEqual

PR-URL: #8590
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
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

test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants