Skip to content
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: changed equality comparison to identity operator #12405

Closed
wants to merge 1 commit into from

Conversation

fcampinho
Copy link
Contributor

@fcampinho fcampinho commented Apr 13, 2017

test: changed equality comparison to identity operator

Changed the equality comparison from == to identity operator ===

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows [commit guidelines]

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 13, 2017
@mscdex mscdex added the crypto Issues and PRs related to the crypto subsystem. label Apr 13, 2017
@vsemozhetbyt
Copy link
Contributor

Copy link
Member

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

@Trott
Copy link
Member

Trott commented Apr 13, 2017

Minor nit for whoever lands this (or if @fcampinho wants to fix it up and force push, that's cool too): First word of the commit message should be an imperative verb, so changed should be change.

@Trott
Copy link
Member

Trott commented Apr 13, 2017

Minor nit part 2: The first line should be 50 chars max so maybe test: change == to === in crypto test or something like that

Changed the equality comparison from == to identity operator ===
@fcampinho
Copy link
Contributor Author

fcampinho commented Apr 13, 2017

Sorry @Trott , first commit, I executed a git push --force

Trott pushed a commit to Trott/io.js that referenced this pull request Apr 16, 2017
Changed the equality comparison from == to identity operator ===

PR-URL: nodejs#12405
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@Trott
Copy link
Member

Trott commented Apr 16, 2017

Landed in 7044065

@Trott Trott closed this Apr 16, 2017
@Trott
Copy link
Member

Trott commented Apr 16, 2017

Thanks for the contribution! 🎉

@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Should land after #11705

MylesBorins pushed a commit that referenced this pull request Oct 16, 2017
Changed the equality comparison from == to identity operator ===

PR-URL: #12405
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 17, 2017
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
Changed the equality comparison from == to identity operator ===

PR-URL: #12405
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.