Skip to content
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

[fix] Avoid default_app_config Deprecation Warning in Django >= 3.2 #268

Merged
merged 4 commits into from Jan 12, 2022

Conversation

bluppfisk
Copy link
Contributor

This will stop Django 3.2 and higher from raising Deprecation Warnings because default_app_config is no longer necessary and will be removed in 4.1. Note that it breaks backward incompatibility with Django < 3.2 so it should only be used in releases dropping <3.2 support.

Yes, It may be a bit early for this one :)

Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluppfisk
Copy link
Contributor Author

done :)

@auvipy
Copy link
Collaborator

auvipy commented Jan 4, 2022

please check the build failures

@bluppfisk bluppfisk requested a review from auvipy January 4, 2022 14:15
@auvipy
Copy link
Collaborator

auvipy commented Jan 5, 2022

  • missing prefix in the commit short description
    Eg: "[feature/fix/change] Action performed"

Please read our guidelines at: http://openwisp.io/docs/developer/contributing.html#commit-message-style-guidelines
ERROR: Commit message check failed!
Checked commit message:

@auvipy auvipy changed the title remove default_app_config which is deprecated in Django 4.1 [fix] remove default_app_config which is deprecated in Django 4.1 Jan 5, 2022
Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to change the commit format

@bluppfisk
Copy link
Contributor Author

ok tried my best :)

Django 3.2 started throwing deprecation warnings about the use of
default_app_config in modules' __init__.py. The practice will start
throwing errors in Django 4.1. This precludes future problems
by only including the code if the Django version is below 3.2.
@auvipy
Copy link
Collaborator

auvipy commented Jan 9, 2022

I had to fixed a merge conflict, can you please pull & run flake8 check and push again?

@auvipy
Copy link
Collaborator

auvipy commented Jan 12, 2022

Your commit message does not follow our commit message style guidelines:

  • please add a capital letter after the prefix

Please read our guidelines at: http://openwisp.io/docs/developer/contributing.html#commit-message-style-guidelines
Checked commit message:

[fix] flake8 formatting
ERROR: Commit message check failed!

@auvipy auvipy changed the title [fix] remove default_app_config which is deprecated in Django 4.1 [fix] Remove default_app_config which is deprecated in Django 4.1 Jan 12, 2022
@auvipy auvipy changed the title [fix] Remove default_app_config which is deprecated in Django 4.1 [fix] Avoid default_app_config Deprecation Warning in Django >= 3.2 Jan 12, 2022
@auvipy auvipy merged commit 29228a4 into openwisp:master Jan 12, 2022
@auvipy
Copy link
Collaborator

auvipy commented Jan 12, 2022

strange, it was green in the PR & showing failure after merge!

@bluppfisk
Copy link
Contributor Author

Not sure why. The QA checks seem very strict for this project. Not necessarily a bad thing, but it looks like it thinks I've mentioned a particular issue (I didn't) and is angry at me for not mentioning that it is fixed in this PR. At least the code should work fine.

pandafy added a commit that referenced this pull request Jan 12, 2022
pandafy added a commit that referenced this pull request Jan 12, 2022
nemesifier pushed a commit that referenced this pull request Jan 12, 2022
@nemesifier
Copy link
Member

The problem was fixed in #270 and it was not a strictly QA check issue as far as I understood.
@bluppfisk QA checks enforce consistency, we do this across all openwisp modules and it helps maintainers like me to maintain a lot of different packages faster because I can find things more easily.

@batisteo
Copy link

Would it be possible to have a minor release with this fix? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants