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

util: cleanup util #25255

Closed
wants to merge 5 commits into from
Closed

util: cleanup util #25255

wants to merge 5 commits into from

Conversation

BridgeAR
Copy link
Member

Just some cleanup while trying to figure out some other things.

Please have a look at the commit messages for details.

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 makes sure the counter goes up instead of going down. This allows
to properly track the current inspection depth no matter what the
`depth` option was set to.
Remove some dead code plus some minor refactoring for readability.
The constructor can not be an empty string anymore, so just remove
that check.
This removes a special casing for this data type in the main function.
This comment is not correct anymore.
This should improve the readability of the code.
@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Dec 28, 2018
@BridgeAR
Copy link
Member Author

BridgeAR commented Jan 9, 2019

@nodejs/util PTAL

This is open for quite some while without getting any reviews.

@BridgeAR
Copy link
Member Author

BridgeAR commented Jan 9, 2019

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 9, 2019
@BridgeAR
Copy link
Member Author

@danbev
Copy link
Contributor

danbev commented Jan 11, 2019

Landed in eca2760, 76fa37a, 1ab659a, 1bee544, 6cc74b0.

@danbev danbev closed this Jan 11, 2019
danbev pushed a commit that referenced this pull request Jan 11, 2019
This makes sure the counter goes up instead of going down. This allows
to properly track the current inspection depth no matter what the
`depth` option was set to.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
danbev pushed a commit that referenced this pull request Jan 11, 2019
Remove some dead code plus some minor refactoring for readability.
The constructor can not be an empty string anymore, so just remove
that check.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
danbev pushed a commit that referenced this pull request Jan 11, 2019
This removes a special casing for this data type in the main function.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
danbev pushed a commit that referenced this pull request Jan 11, 2019
This comment is not correct anymore.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
danbev pushed a commit that referenced this pull request Jan 11, 2019
This should improve the readability of the code.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
This makes sure the counter goes up instead of going down. This allows
to properly track the current inspection depth no matter what the
`depth` option was set to.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
Remove some dead code plus some minor refactoring for readability.
The constructor can not be an empty string anymore, so just remove
that check.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
This removes a special casing for this data type in the main function.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
This comment is not correct anymore.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
This should improve the readability of the code.

PR-URL: #25255
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
This makes sure the counter goes up instead of going down. This allows
to properly track the current inspection depth no matter what the
`depth` option was set to.

PR-URL: nodejs#25255
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
Remove some dead code plus some minor refactoring for readability.
The constructor can not be an empty string anymore, so just remove
that check.

PR-URL: nodejs#25255
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
This removes a special casing for this data type in the main function.

PR-URL: nodejs#25255
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
This comment is not correct anymore.

PR-URL: nodejs#25255
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
This should improve the readability of the code.

PR-URL: nodejs#25255
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
@BridgeAR BridgeAR deleted the cleanup-util branch January 20, 2020 11:46
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. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants