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

lib: use strict equality in _http_server and _tls_wrap #9849

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@walterbm
Contributor

walterbm commented Nov 30, 2016

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

lib/_http_server
lib/_tls_wrap

Description of change

Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

@Trott

Trott approved these changes Nov 30, 2016

LGTM if CI is

If you can, could you change the commit message first line to be no more than 50 chars? Maybe something like lib: use === in _http_server and _tls_wrap

@Trott

This comment has been minimized.

Show comment
Hide comment
lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.
@walterbm

This comment has been minimized.

Show comment
Hide comment
@walterbm

walterbm Nov 30, 2016

Contributor

@Trott changed the first line of commit message first line to be less than 50 characters.

Seems like CI is failing for the ARM build of Node but I'm having some trouble locating the error. Looking through the Jenkins console log I can see a build failure but I'm not sure if that is the root cause. I couldn't find any failing tests.

Contributor

walterbm commented Nov 30, 2016

@Trott changed the first line of commit message first line to be less than 50 characters.

Seems like CI is failing for the ARM build of Node but I'm having some trouble locating the error. Looking through the Jenkins console log I can see a build failure but I'm not sure if that is the root cause. I couldn't find any failing tests.

@mhdawson

LGTM

@gibfahn

This comment has been minimized.

Show comment
Hide comment
Member

gibfahn commented Nov 30, 2016

@mhdawson

This comment has been minimized.

Show comment
Hide comment
@mhdawson

mhdawson Nov 30, 2016

Member

This is the output for the failure: https://ci.nodejs.org/job/node-test-binary-arm/4915/RUN_SUBSET=addons,label=pi1-raspbian-wheezy/console

Looks unrelated to change being made here.

Member

mhdawson commented Nov 30, 2016

This is the output for the failure: https://ci.nodejs.org/job/node-test-binary-arm/4915/RUN_SUBSET=addons,label=pi1-raspbian-wheezy/console

Looks unrelated to change being made here.

@gibfahn

This comment has been minimized.

Show comment
Hide comment
@gibfahn

gibfahn Nov 30, 2016

Member

Rerun of CI was green!

Member

gibfahn commented Nov 30, 2016

Rerun of CI was green!

@targos

targos approved these changes Dec 1, 2016

@lpinca

lpinca approved these changes Dec 1, 2016

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

lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

PR-URL: nodejs#9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Dec 3, 2016

Member

Landed in 20fa6e7.

Thanks, @walterbm! 🎉

Member

Trott commented Dec 3, 2016

Landed in 20fa6e7.

Thanks, @walterbm! 🎉

@Trott Trott closed this Dec 3, 2016

addaleax added a commit that referenced this pull request Dec 5, 2016

lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

PR-URL: #9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@Fishrock123 Fishrock123 referenced this pull request Dec 5, 2016

Merged

v7.2.1 proposal #10127

2 of 2 tasks complete

jmdarling added a commit to jmdarling/node that referenced this pull request Dec 8, 2016

lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

PR-URL: nodejs#9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 20, 2016

lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

PR-URL: #9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 21, 2016

lib: use === in _http_server and _tls_wrap
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.

PR-URL: #9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@MylesBorins MylesBorins referenced this pull request Dec 21, 2016

Merged

V6.9.3 proposal #10394

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