Skip to content

Conversation

lawik
Copy link

@lawik lawik commented Mar 10, 2014

Fixes my problems reported in #111.

Validation is taken from Django URLValidator in 1.6.2 and http://stackoverflow.com/a/2532344 .

@lawik
Copy link
Author

lawik commented Mar 11, 2014

Looks like it has problems running on django1.4 if I understand the CI output correctly. What is the version support strategy here?

@lawik
Copy link
Author

lawik commented Mar 11, 2014

@masci
Copy link
Contributor

masci commented Mar 15, 2014

For django versions >=1.5 tests are failing because of test_validate_bad_uris- you should fix that testcase accordingly with your changes.

Regarding django 1.4 (which we want to support since it is a LTS release), at the moment v. 1.4.10 provides an old version of six, so you shouldn't use it in validators.py module. You could instead add needed imports in compat.py module and import from there.

As a side note, according to our contribution guidelines, for future PRs please ask for merging from a feature branch instead of your master to avoid rebasing when our master goes on.

Don't forget to add your name to the AUTHORS file, and thanks for your code!

@lawik
Copy link
Author

lawik commented Mar 17, 2014

I'll try to get more time to clean this up for compatibility. My need is only with 1.6 but I love and respect backwards compatiblity.

I was not aware of any contributing guidelines until you mentioned them just now. I did check the first page, but now I found them in the CONTRIBUTE.rst: https://django-oauth-toolkit.readthedocs.org/en/latest/contributing.html
Appreciate the response and info :)

@masci masci merged commit c6f7c20 into django-oauth:master Jul 3, 2014
@masci masci added this to the 0.8.0 milestone Jul 3, 2014
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.

2 participants