-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
BREAKING CHANGE: Bump version of normalize-url to 8, dependency bumps #87
Conversation
@@ -33,13 +33,17 @@ | |||
"string" | |||
], | |||
"dependencies": { | |||
"normalize-url": "^7.0.2", | |||
"re2": "^1.17.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re2
isn't used anywhere in the project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's required by url-regex-safe
. Usually, git blame
is a good way to find out why something exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, will add it back. Requiring a build step for this library feels a bit too much - I might open up an issue for discussion on using an alternate implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we can switch to https://github.com/sindresorhus/super-regex instead. Feel free to do that here.
.gitignore
Outdated
@@ -1,2 +1,3 @@ | |||
node_modules | |||
yarn.lock | |||
.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't make unrelated changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my IDEs config files, happy to remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's your config. It has nothing to do in a repo. It should be in your own global gitignore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
@sindresorhus |
I will require Node.js 16 after merging this. |
Sounds good! I think PR is ready now |
Bumps dependency versions, fixes lint errors
There's a breaking change in normalize-url v7.0.3 around url query strings - sindresorhus/normalize-url#158 which I figured should be a major version bump