-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Added regex to handle url parts #603
Conversation
Congratulations 🍻. DeepCode analyzed your code in 0.618 seconds and we found no issues. Enjoy a moment of no bugs ☀️. 💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues. |
Hey @JacobAnavisca, TravisCI finished with status TravisBuddy Request Identifier: c3615f20-579a-11ea-acab-ef795e2c3319 |
Hey @JacobAnavisca, TravisCI finished with status TravisBuddy Request Identifier: bc2c0e60-579c-11ea-acab-ef795e2c3319 |
@JacobAnavisca there seems to be a problem with parsing URL while passing |
@JacobAnavisca pre-commit hooks have been introduced hence please rebase to current master and pull latest change, then send PR. run |
Hey @JacobAnavisca, TravisCI finished with status TravisBuddy Request Identifier: f1a14720-57e4-11ea-acab-ef795e2c3319 |
I don't think we should be using (our own) regex for this. JavaScript has a very mature URL API: |
main reason I'm using regex is to handle environment variables, like << baseUrl >>/get |
Could we substitute them before we try parsing? |
How about regex as a fallback? like so: |
Hey @JacobAnavisca, TravisBuddy Request Identifier: adcf5c10-584f-11ea-b533-891a36a4ac0b |
Hey @JacobAnavisca, TravisBuddy Request Identifier: 851805f0-5855-11ea-b533-891a36a4ac0b |
Hey @JacobAnavisca, TravisBuddy Request Identifier: 228e7830-5858-11ea-b533-891a36a4ac0b |
Hey @JacobAnavisca, TravisBuddy Request Identifier: 7c375050-5871-11ea-b533-891a36a4ac0b |
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.
LGTM
Fix #62
Fix #581