Django 3.1 support #6886
Django 3.1 support #6886
Conversation
FinalAngel
commented
Aug 6, 2020
•
FinalAngel
commented
- Added support for Django 3.1
- Dropped support for Python 2.7 and Python 3.4
- Dropped support for Django < 2.2
|
@FinalAngel it looks straighforward to me 👍 |
| @@ -59,7 +57,7 @@ def _verify_apphook(apphook, namespace): | |||
| apphook_name = apphook.__class__.__name__ | |||
| elif hasattr(apphook, '__module__') and issubclass(apphook, CMSApp): | |||
| return apphook.__name__ | |||
| elif isinstance(apphook, string_types): | |||
| elif isinstance(apphook, str): | |||
There was a problem hiding this comment.
I'm not sure if this is ok for Strings, I know you can't do this with list because Strings can be True when checking against a type of List.
There was a problem hiding this comment.
string_types is equivalent to str in Python 3
| @@ -9,7 +9,7 @@ | |||
| import django | |||
| from django.contrib.admin.helpers import AdminForm | |||
| from django.conf import settings | |||
| from django.conf.urls import url | |||
| from django.urls import re_path | |||
There was a problem hiding this comment.
I guess to ensure that the codebase worked as it did re_path is the same as url was, it is recommended to switch out to path but I think migrations like this should be staged and maybe pencil something in for a future change to use path vs re_path.
There was a problem hiding this comment.
yeah some could potentially be migrated to django.urls.path but in this case we just want to be sure
|
|
||
| class StaticFilesTest(TestCase): | ||
|
|
||
| def test_collectstatic_with_cached_static_files_storage(self): |
There was a problem hiding this comment.
@Aiky30 it tests for django.contrib.staticfiles.storage.CachedStaticFilesStorage which has been removed in Django 3.1
cms/admin/placeholderadmin.py
Outdated
| @@ -24,8 +26,6 @@ | |||
| from django.views.decorators.clickjacking import xframe_options_sameorigin | |||
| from django.views.decorators.http import require_POST | |||
|
|
|||
| from six.moves.urllib.parse import parse_qsl, urlparse | |||
|
|
|||
| from six import get_unbound_function, get_method_function | |||
There was a problem hiding this comment.
From what I can see in the diff six looks to be installed just for these two imports, I assume logic that was for 2 that needs to still work in 3.
There was a problem hiding this comment.
six is now fully removed :)
|
Just dropping a comment here, I need to investigate further
|
|
Sure, I'm entirely neutral on this
I'm fine with either solution
24 Aug 2020 09:38:39 Angelo Dini <notifications@github.com>:
… @FinalAngel commented on this pull request.
----------------------------------------
In setup.py[#6886 (comment)]:
> import os -import cms + +from cms import __version__ +from setuptools import find_packages, setup + + +REQUIREMENTS = [ + ***@***.***[https://github.com/yakky] true but on the other hand restricting doesn't gain much either. That way people can try out django/master earlier without locally modifying the files.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub[#6886 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAFOPV7NOHQCRMJGNA7VEA3SCIKHNANCNFSM4PWUTJTQ]. [https://s3.amazonaws.com/msv5/images/spacer.gif]
[ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#6886 (comment)", "url": "#6886 (comment)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ]
|
|
@yakky wehee a passing build :D |
|
@FinalAngel 👍 from me |
