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: remove unused args and comparison fix. #11396

Closed
wants to merge 1 commit into from
Closed

test: remove unused args and comparison fix. #11396

wants to merge 1 commit into from

Conversation

sashashakun
Copy link
Contributor

@sashashakun sashashakun commented Feb 15, 2017

Remove unused arguments and change non-strict comparison
to the strict one.

/cc @Trott

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

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 15, 2017
@mscdex mscdex added the dgram Issues and PRs related to the dgram subsystem / UDP. label Feb 15, 2017
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.

Changes LGTM. Can you format the commit message according to our guidelines?

@sashashakun
Copy link
Contributor Author

@targos thanks for answer. I add subsystem name to my commit message and check length and format of the message, as I see it follow the guidelines. What I missed?

@targos
Copy link
Member

targos commented Feb 15, 2017

test (all lowercase) was the correct subsystem

Remove unused arguments and change non-strict comparison
to the strict one in test.
@sashashakun sashashakun changed the title Test: remove unused args and comparison fix. test: remove unused args and comparison fix. Feb 15, 2017
@hiroppy
Copy link
Member

hiroppy commented Feb 15, 2017

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

@sashashakun
Copy link
Contributor Author

Not sure what happens, as I see here all CI jobs are finished successfully. What I miss?

@targos
Copy link
Member

targos commented Feb 16, 2017

@sashashakun Nothing to worry about. There is a bug with the reporter for test/arm results.

Trott pushed a commit to Trott/io.js that referenced this pull request Feb 17, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: nodejs#11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member

Trott commented Feb 17, 2017

Landed in 55d202a.
Thanks for the contribution! 🎉

@Trott Trott closed this Feb 17, 2017
@Trott Trott reopened this Feb 17, 2017
@Trott Trott closed this Feb 17, 2017
@sashashakun sashashakun deleted the small-fixes-in-test branch February 17, 2017 19:53
addaleax pushed a commit that referenced this pull request Feb 22, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: #11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@italoacasas italoacasas mentioned this pull request Feb 25, 2017
jasnell pushed a commit that referenced this pull request Mar 7, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: #11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: #11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: #11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Remove unused arguments and change non-strict comparison
to the strict one in test.

PR-URL: #11396
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants