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

domain: do not import util for a simple type check #29825

Closed
wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Oct 3, 2019

This removes require('util') from the domain module. There was
only a single simple type check used from the util module which
is now inlined instead.

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

This removes `require('util')` from the `domain` module. There was
only a single simple type check used from the `util` module which
is now inlined instead.
@nodejs-github-bot nodejs-github-bot added the domain Issues and PRs related to the domain subsystem. label Oct 3, 2019
lib/domain.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

Trott
Trott previously requested changes Oct 11, 2019
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.

Needs parentheses (or other change) to pass linter. Otherwise, no objections and this "request changes" can be dismissed.

@Trott Trott dismissed their stale review October 13, 2019 03:14

parentheses added

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 13, 2019
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Oct 13, 2019

Landed in fce1a51

@Trott Trott closed this Oct 13, 2019
Trott pushed a commit that referenced this pull request Oct 13, 2019
This removes `require('util')` from the `domain` module. There was
only a single simple type check used from the `util` module which
is now inlined instead.

PR-URL: #29825
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
targos pushed a commit that referenced this pull request Nov 8, 2019
This removes `require('util')` from the `domain` module. There was
only a single simple type check used from the `util` module which
is now inlined instead.

PR-URL: #29825
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
This removes `require('util')` from the `domain` module. There was
only a single simple type check used from the `util` module which
is now inlined instead.

PR-URL: #29825
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@BridgeAR BridgeAR deleted the remove-require-util branch January 20, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. domain Issues and PRs related to the domain subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants