search

mozilla / django-browserid

Django application for adding BrowserID support.
Mozilla Public License 2.0
180 stars 80 forks source link

Fix #195: Add support for Python 3.2 and 3.3 and enable tests on them. #204

Closed Osmose closed 10 years ago

Osmose commented 10 years ago

Thanks to @senden9 for most of these fixes. Rebasing #187 proved to be more work than just rewriting the patch to the new code.

Osmose commented 10 years ago

@jezdez r?

jezdez commented 10 years ago

I would strongly recommend using Django's vendored version of six, instead of the one from PyPI. Replace six imports with from django.utils import six.

jezdez commented 10 years ago

Other than that, this looks good. Not sure if it needs to be rebased.

Osmose commented 10 years ago

Added a commit with the review fixes. A re-r? would be helpful given the size. :D

The decorator you mentioned is only available in Django 1.4 and up, and given that 1.3 is old and we already have a few other workarounds for it, it's time we nixxed it, so I also removed 1.3 support.

@peterbe, @willkg: Are you okay with dropping 1.3 support now?

jezdez commented 10 years ago

FTR, 1.3.x is not even covered by the security policy anymore -- while 1.4.x is going to be for a while due to its LTS status. https://docs.djangoproject.com/en/dev/internals/security/#supported-versions

willkg commented 10 years ago

Sorry about that. I thought @Osmose was asking me to look at those two commits, not at the whole pr. Sorry for wasting everyone's time.

willkg commented 10 years ago

I didn't read through all the tests, but I did skim them, though I'm not sure how helpful that really is. This looks ok to me as far as I've read through it.

Osmose commented 10 years ago

I didn't read through all the tests, but I did skim them, though I'm not sure how helpful that really is. This looks ok to me as far as I've read through it.

Good enough for me!