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 util imports #4397

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 23, 2015

A number of tests in test/parallel were importing the util module
via require() but not using util for anything. This removes those
require() statements.

A number of tests in `test/parallel` were importing the `util` module
via `require()` but not using `util` for anything. This removes those
`require()` statements.
@Trott Trott added the test Issues and PRs related to the tests. label Dec 23, 2015
@rvagg
Copy link
Member

rvagg commented Dec 23, 2015

@cjihrig
Copy link
Contributor

cjihrig commented Dec 23, 2015

LGTM

Trott added a commit to Trott/io.js that referenced this pull request Dec 25, 2015
A number of tests in `test/parallel` were importing the `util` module
via `require()` but not using `util` for anything. This removes those
`require()` statements.

PR-URL: nodejs#4397
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@Trott
Copy link
Member Author

Trott commented Dec 25, 2015

Landed in 3311267

@Trott Trott closed this Dec 25, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
A number of tests in `test/parallel` were importing the `util` module
via `require()` but not using `util` for anything. This removes those
`require()` statements.

PR-URL: nodejs#4397
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@MylesBorins
Copy link
Contributor

I seems like some of these "unused" utils are still be used in LTS. @Trott do you want to open a different PR to backport this?

@Trott
Copy link
Member Author

Trott commented Jan 7, 2016

@thealphanerd want is a strong word, but I suppose I can do it. So, uh, how? Do I just open a PR against some branch that isn't master? Or what?

@Trott
Copy link
Member Author

Trott commented Jan 7, 2016

Ah, I see in another issue you specify v4.x-staging. Sure, I'll get to that.

@Trott
Copy link
Member Author

Trott commented Jan 7, 2016

@thealphanerd: Here you go: #4562

scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
A number of tests in `test/parallel` were importing the `util` module
via `require()` but not using `util` for anything. This removes those
`require()` statements.

PR-URL: nodejs#4397
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@Trott Trott deleted the no-unused-vars branch January 13, 2022 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants