-
Notifications
You must be signed in to change notification settings - Fork 259
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
Django 1.7 compatibility #78
base: master
Are you sure you want to change the base?
Conversation
+ fallback for Django<1.7
#77 probably affects other parts of the code base as well (e.g. views.py#L301 ), consider moving the (I was sent here to review this pull request by http://www.codevolley.com/ , please ignore my comments if they are out of place :) ) |
any news on this? :) |
@luto I bumped into a lot of problems with migrations in 1.7, so downgraded to 1.6 again. I might spend time on this again once 1.7 is released. |
@sigvef The current PR addresses views.py#L301 and I can't find another affected place in the codebase. It also works well for me. 👍 for merging it as 1.7 is now final. :) I have a fix for |
@@ -9,6 +8,14 @@ | |||
from . import constants, scope | |||
|
|||
|
|||
try: | |||
from django.http import JsonResponse |
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.
great, I would add since >= django1.7 comment and maybe move that to provider/compat
?
Hi, has this been abandoned? looks like this PR is still open |
@egamonal I think https://github.com/idan/oauthlib is a safer bet these days :) |
thanks @antonagestam , I was trying django-oauth-toolkit to get ready for updating django rest framework to 3.x . I'll give that one a shot as well. |
fixes #77
Replaced
django.http.HttpResponse
withdjango.http.JsonResponse
with fallback for Django < 1.7.