-
Notifications
You must be signed in to change notification settings - Fork 119
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
remove hypothesis (at least for now) #586
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #586 +/- ##
==========================================
+ Coverage 99.04% 99.05% +0.01%
==========================================
Files 44 45 +1
Lines 3887 3935 +48
Branches 520 523 +3
==========================================
+ Hits 3850 3898 +48
Misses 23 23
Partials 14 14
☔ View full report in Codecov by Sentry. |
yield DecodedURL.from_text("https://example.com") | ||
yield DecodedURL.from_text("http://example.com/") | ||
yield DecodedURL.from_text("https://example.com/é") | ||
yield DecodedURL.from_text("https://súbdomain.example.com/ascii/path/") |
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.
Do you want a URL str that has a %-encoded path element here?
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.
As a basic compatibility layer this seems like a reasonable implementation.
Thanks @dreid . Still need to get the robot to respect non-core reviews. |
Fixes #561
I am not entirely convinced we need Hypothesis at all, but if we do, when we reintroduce it we have to first do the work to make it deterministic in CI and to address the various deficiencies in Hyperlink's strategies if we want to use those.
(ugh I did a thing in the wrong tab, please ignore the other PR)