Skip to content

Conversation

@rakanalh
Copy link

@rakanalh rakanalh commented May 9, 2015

This PR is a rebase of @avendael's work on Python3 compatibility to the latest version of develop.

Thank you @geeknam, @avendael

@coveralls
Copy link

Coverage Status

Coverage decreased (-9.49%) to 81.92% when pulling 10e5163 on rakanalh:master into 0af0b12 on geeknam:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.99%) to 85.42% when pulling e731ecf on rakanalh:master into 0af0b12 on geeknam:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 7% when pulling e731ecf on rakanalh:master into 0af0b12 on geeknam:master.

@rakanalh
Copy link
Author

@geeknam did you get the chance to review this?

@dotpot
Copy link

dotpot commented May 13, 2015

👍 I was thinking about doing similar PR. :))

@geeknam
Copy link
Owner

geeknam commented May 27, 2015

Thanks guys, can we get the coverage & health up? Who wants to volunteer to do this? :)

@rakanalh
Copy link
Author

I believe the priority should be making a release rather than waiting for people to improve health indicator & tests. All current tests pass so the library should be good to go for a release with this branch.

This is my 0.02

@geeknam
Copy link
Owner

geeknam commented May 27, 2015

Thanks for your input @rakanalh. Landscape caught an undefined variable urllib https://landscape.io/diff/148815

@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling d6d1e55 on rakanalh:master into 0af0b12 on geeknam:master.

@geeknam
Copy link
Owner

geeknam commented May 27, 2015

In my opinion, using requests and replacing urllib and urllib2 will definitely be a good move here.

Benefits:

  • requests is python 3 compatible
  • using requests would make the code cleaner
  • everyone is familiar with requests, requests is more human friendly

@rakanalh
Copy link
Author

I agree... but many people along with myself need this to be Py3 compliant as soon as possible.

As for requests, i also agree. I prefer using requests over urllib. Could you leave this as an open issue for someone to pick up and create a PR for that?

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.66%) to 85.76% when pulling d6d1e55 on rakanalh:master into 0af0b12 on geeknam:master.

@geeknam
Copy link
Owner

geeknam commented May 29, 2015

#60 fixes this

@geeknam geeknam closed this May 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants