-
Notifications
You must be signed in to change notification settings - Fork 364
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 GeoTrellisPath parsing #3191
Fix GeoTrellisPath parsing #3191
Conversation
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; @CloudNiner you can also try to make schemelss URIs work and if none of the other tests would fail lets merge it 💯
9d6356d
to
d8764b0
Compare
With fix applied:
🎉 |
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 👍 merging once travis is happy!
Overview
Parsing of url authority was bad. This should fix that. This PR also opened a discussion around whether absolute and relative urls without scheme should be valid as well so there's tests for those here that currently fail. If we want to support this, I can add a fix for that here as well, where scheme-less urls assume
file://
.Before fix, 7 of new tests fail. After, only the scheme-less tests fail.
Checklist
docs
guides update, if necessary