New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: stricter comparisons for equal and require var to const #8472

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@HyperSprite
Contributor

HyperSprite commented Sep 9, 2016

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

test net

Description of change

Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Sep 9, 2016

Member

LGTM if CI doesn't reveal any surprises.

Member

Trott commented Sep 9, 2016

LGTM if CI doesn't reveal any surprises.

@Trott

This comment has been minimized.

Show comment
Hide comment
@cjihrig

This comment has been minimized.

Show comment
Hide comment
@cjihrig

cjihrig Sep 9, 2016

Contributor

LGTM

Contributor

cjihrig commented Sep 9, 2016

LGTM

@lpinca

This comment has been minimized.

Show comment
Hide comment
@lpinca

lpinca Sep 10, 2016

Member

CI failure (AIX) seems unrelated, LGTM.

@HyperSprite Would you mind updating your commit message and make it 50 chars or less? Thanks!

Member

lpinca commented Sep 10, 2016

CI failure (AIX) seems unrelated, LGTM.

@HyperSprite Would you mind updating your commit message and make it 50 chars or less? Thanks!

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Sep 10, 2016

Member

(Just a clarification: the 50 chars or less refers to the first line of the commit message only. Subsequent lines should wrap at 72 chars.)

Member

Trott commented Sep 10, 2016

(Just a clarification: the 50 chars or less refers to the first line of the commit message only. Subsequent lines should wrap at 72 chars.)

@thefourtheye

This comment has been minimized.

Show comment
Hide comment
@thefourtheye

thefourtheye Sep 10, 2016

Contributor

LGTM

Contributor

thefourtheye commented Sep 10, 2016

LGTM

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

Trott added a commit to Trott/io.js that referenced this pull request Sep 13, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: nodejs#8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Sep 13, 2016

Member

Landed in 77e9679

Member

Trott commented Sep 13, 2016

Landed in 77e9679

@Trott Trott closed this Sep 13, 2016

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: #8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

MylesBorins added a commit that referenced this pull request Sep 30, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: #8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

MylesBorins added a commit that referenced this pull request Oct 10, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: #8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

rvagg added a commit that referenced this pull request Oct 18, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: #8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

MylesBorins added a commit that referenced this pull request Oct 26, 2016

test: swapped == and equal to === and strictEqual
Line 21 used '==' for a Number comparison, changed to '===''.
Lines 36 and 46 used 'assert.equal', changed to 'assert.strictEqual'.
Lines 2, 3 and 4 require statements used var, changed to const.

PR-URL: #8472
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

@MylesBorins MylesBorins referenced this pull request Oct 26, 2016

Closed

V4.6.2 proposal #9298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment