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
use requests instead of urllib #89
Conversation
(1) |
Oh wow! I was idly wishing in that comment. This is pretty nice. For tldextract 1.x, I'd like to stick to no external dependencies. Mind if I wait to merge this until 2.x? (Concretely, #81 is expected to force a 2.0 release.) |
@@ -5,4 +5,6 @@ pytest==2.7.3 | |||
pytest-gitignore==1.3 | |||
pytest-mock==0.9.0 | |||
pytest-pylint==0.4.0 | |||
requests>=2.9.1 |
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.
Should this be >= 2.0, < 3
? Maybe I've been in Node.js too long.
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.
Does this need to be in setup.py too?
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.
Thanks!
no problem, let's keep this open until 2.x |
Good job @jnozsc |
use requests instead of urllib Conflicts: tldextract/tldextract.py
This PR can be closed, right? It is already merged in 2.X branch :) |
Never mind manually closing this. Keeping this open until autoclose on |
Hi all, Is pinning I ran the test suite with requests 2.2.1, requests 2.8.1, and requests 2.9.1 and they all pass. |
The exact version range was somewhat arbitrary. I think it can be more flexible. Thanks for #98! Let's discuss there. |
as #88 (comment) mentioned