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

fix: remove unused broken require #1661

Closed
wants to merge 2 commits into from
Closed

fix: remove unused broken require #1661

wants to merge 2 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Aug 12, 2020

References

Fixes: #1658

@aduh95 aduh95 requested a review from a team as a code owner August 12, 2020 08:52
@MylesBorins
Copy link
Contributor

Oh I totally missed this, I just opened a dupe

#1665

Based on the patterns in npm (and the fact that people use it on versions of node > 10.x I think that the solution I proposed is a bit better)... closing my PR and making a suggested change ehre

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM

@claudiahdz
Copy link
Contributor

Just a heads up to @ruyadorno, this fix should include updating npm-registry-fetch to v.4.0.6 that had the same bug.

@ruyadorno ruyadorno added Release 6.x work is associated with a specific npm 6 release semver:patch semver patch level for changes labels Aug 14, 2020
ruyadorno pushed a commit that referenced this pull request Aug 17, 2020
Fixes: #1658

PR-URL: #1661
Credit: @aduh95
Close: #1661
Reviewed-by: @ruyadorno
ruyadorno pushed a commit that referenced this pull request Aug 17, 2020
Fixes: #1658

PR-URL: #1661
Credit: @aduh95
Close: #1661
Reviewed-by: @ruyadorno
@ruyadorno ruyadorno closed this in 376bc08 Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release 6.x work is associated with a specific npm 6 release semver:patch semver patch level for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: URL is not a constructor in replace-info.js
4 participants