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 unsused arguments from function #14931

Closed
wants to merge 2 commits into
base: master
from

Conversation

@ankitiitb1069
Contributor

ankitiitb1069 commented Aug 18, 2017

Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
test
test: remove unsused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js
@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex Aug 18, 2017

Contributor

Looks like there is a 'venv' directory that was incorrectly included in the commit and should be removed.

Contributor

mscdex commented Aug 18, 2017

Looks like there is a 'venv' directory that was incorrectly included in the commit and should be removed.

@mscdex mscdex added http test labels Aug 18, 2017

@ankitiitb1069

This comment has been minimized.

Show comment
Hide comment
@ankitiitb1069

ankitiitb1069 Aug 18, 2017

Contributor

removed that with the latest commit :)

Contributor

ankitiitb1069 commented Aug 18, 2017

removed that with the latest commit :)

@Trott

Trott approved these changes Aug 18, 2017

Hello, @ankitiitb1069! Thanks for the PR! LGTM if CI run is successful.

@Trott

This comment has been minimized.

Show comment
Hide comment
@refack

refack approved these changes Aug 19, 2017

@refack

This comment has been minimized.

Show comment
Hide comment
@refack

refack Aug 19, 2017

Member

Hello @ankitiitb1069 and welcome, GitHub is indicating this is your first contribution, so just in case you havn't already, it's recommended to take a look at the CONTRIBUTING.md guide (especialy the part about "discuss and update").
PRs are left open for at least 48 hour so that anyone interested gets a chance to comment or review.

Anyway thank you for the contribution, and good luck.

Member

refack commented Aug 19, 2017

Hello @ankitiitb1069 and welcome, GitHub is indicating this is your first contribution, so just in case you havn't already, it's recommended to take a look at the CONTRIBUTING.md guide (especialy the part about "discuss and update").
PRs are left open for at least 48 hour so that anyone interested gets a chance to comment or review.

Anyway thank you for the contribution, and good luck.

@lpinca

lpinca approved these changes Aug 19, 2017

@tniessen tniessen self-assigned this Aug 20, 2017

@aqrln

aqrln approved these changes Aug 22, 2017

@aqrln

This comment has been minimized.

Show comment
Hide comment
@aqrln

aqrln Aug 22, 2017

Member

Hi @ankitiitb1069 and thanks for your contribution! I have started a fresh CI run since node-test-commit-osx and node-test-commit-linux-fips jobs didn't even start running during the old one. Once it finishes, the patch is good to land.

CI: https://ci.nodejs.org/job/node-test-pull-request/9794/

Member

aqrln commented Aug 22, 2017

Hi @ankitiitb1069 and thanks for your contribution! I have started a fresh CI run since node-test-commit-osx and node-test-commit-linux-fips jobs didn't even start running during the old one. Once it finishes, the patch is good to land.

CI: https://ci.nodejs.org/job/node-test-pull-request/9794/

@tniessen

This comment has been minimized.

Show comment
Hide comment
@tniessen

tniessen Aug 23, 2017

Member

Landed in 2a97eb6, thank you for your contribution! 🎉

Member

tniessen commented Aug 23, 2017

Landed in 2a97eb6, thank you for your contribution! 🎉

@tniessen tniessen closed this Aug 23, 2017

tniessen added a commit that referenced this pull request Aug 23, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: #14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

addaleax added a commit to addaleax/ayo that referenced this pull request Aug 25, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: nodejs/node#14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

addaleax added a commit to ayojs/ayo that referenced this pull request Aug 28, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: nodejs/node#14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

MylesBorins added a commit that referenced this pull request Sep 10, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: #14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

@MylesBorins MylesBorins referenced this pull request Sep 10, 2017

Merged

v8.5.0 proposal #15308

MylesBorins added a commit that referenced this pull request Sep 12, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: #14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

MylesBorins added a commit that referenced this pull request Sep 20, 2017

test: remove unused arguments from function
Removed the unused arguments of functions defined in
file test/parallel/test-http-parser.js.

PR-URL: #14931
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

@MylesBorins MylesBorins referenced this pull request Sep 20, 2017

Merged

v6.11.4 proposal #15506

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