-
Notifications
You must be signed in to change notification settings - Fork 22
Unsupport py34; Support newer arrow and requests-oauthlib dependencies #37
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
Conversation
@jlapenna Looks like we should drop support for Python 3.4. Can you make the change? |
I can definitely do that. I've never used worked with travis CI before so it might take me a minute or six to figure out how to stop testing/supporting py3.4 |
Alright (I'm also new to github), I pushed my changes to my fork, which according to the comments here should maybe trigger test re-runs. |
Hm. This seems to still use the tox.ini in your branch, so I'm not sure how to trigger it with this pull request. |
@jlapenna That is right, it just needs to be removed from |
Looks correct, now. yay. I also updated the title of the pull request. |
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.
I have some inline comments (questions) about the 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.
I haven't done a code review in a git pull request before, so I don't know if I resolved your comments correctly. Let me know.
I haven't done a code review in a git pull request before, so I don't know if I resolved your comments correctly. Let me know. |
@brad I have made all the requested 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.
LGTM
|
This also required updating calls to fetch_token to include_client_id, as needed by the withings/nokia API.