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 #4562

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 7, 2016

This is #4397 backported for LTS.

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 lts Issues and PRs related to Long Term Support releases. label Jan 7, 2016
@Trott Trott force-pushed the lts-rm-unused-util branch from 7e2d0e6 to 4468bd4 Compare January 7, 2016 06:09
@Trott
Copy link
Member Author

Trott commented Jan 7, 2016

Let's try that again...

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

@mscdex mscdex added the v4.x label Jan 7, 2016
@@ -2,7 +2,7 @@
var assert = require('assert');
var common = require('../common');

var util = require('util');
var util = require('util');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stylistic change, arguably doesn't belong in a commit with the word 'remove' in it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was an accident. I'll fix it and force push.

@bnoordhuis
Copy link
Member

LGTM with a comment.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 7, 2016

LGTM

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 force-pushed the lts-rm-unused-util branch from 4468bd4 to f323eb7 Compare January 7, 2016 15:32
@jasnell
Copy link
Member

jasnell commented Jan 7, 2016

LGTM

Trott added a commit that referenced this pull request Jan 8, 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: #4562
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 8, 2016

Landed in v4.x-staging in 51e0693

@jasnell jasnell closed this Jan 8, 2016
MylesBorins pushed a commit that referenced this pull request Jan 19, 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: #4562
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
@Trott Trott deleted the lts-rm-unused-util branch January 13, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lts Issues and PRs related to Long Term Support releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants