disqus / disqus-python

Disqus API bindings for Python
https://disqus.com/api/docs/
Apache License 2.0
165 stars 51 forks source link

Now compatible from Python 2.6-3.4 with all unit tests passing. #10

Closed jjangsangy closed 10 years ago

jjangsangy commented 10 years ago

Hi I've refactored most of the incompatibility issues, and package runs on Python 2.6, 2.7, 3.3, and 3.4. Here are some changes i've made.

Build Status

Changelog

Some additional refactoring that would be useful

TODO:

mattrobenolt commented 10 years ago

Whoa. Lemme take some time to review and leave feedback. Thank you. :)

jjangsangy commented 10 years ago

No probs

jjangsangy commented 10 years ago

Hi was wondering if there was any headway on this?

mattrobenolt commented 10 years ago

Hey @jjangsangy, so most of these things are good. But to simplify this, this shoudl really be split into the things that are required to be python3 compatible, then treat each other contribution as a separate thing.

For example, I disagree with a few decisions that are being made, but they're totally outside the scope of the python3 compatibility efforts.

Can we narrow this down to just the minimum efforts for that, then break everything else into it's own PR?

Like, the changes being made to version and Makefile, etc. Those are all completely unrelated and not necessarily something I want to change.

jjangsangy commented 10 years ago

Sure no prob, I can make a new PR for just py3k compatibility and you guys can decide what other stuff to keep.

mattrobenolt commented 10 years ago

All of the python 2->3 still looks good aside from my couple comments. :+1:

jjangsangy commented 10 years ago

Submitted a stripped down pr, so we can probably close this one.

Thanks for all the feedback!

mattrobenolt commented 10 years ago

Cool, yeah, we merged the relevant pieces in another PR. Thanks again.