django / django-contrib-comments

BSD 3-Clause "New" or "Revised" License
614 stars 196 forks source link

Added testing for Django 3.2. #168

Closed felixxm closed 3 years ago

claudep commented 3 years ago

The test failures is weird, indeed! The global apps symbol is django_comments.apps instead of the imported django.apps.apps. @felixxm, any idea what's going on? I bisected the problem to https://github.com/django/django/commit/3f2821af6bc4

felixxm commented 3 years ago

The test failures is weird, indeed! The global apps symbol is django_comments.apps instead of the imported django.apps.apps. @felixxm, any idea what's going on? I bisected the problem to django/django@3f2821a

It's a side-effect of an import_module('django_comments.app') call. In older versions of Django, we have the same side-effect with default_app_config = 'django_comments.apps.DjangoCommentsAdminConfig' defined in django_comments/__init__.py so it's not a regression, IMO.

We could add a warning to the docs that importing django.apps in __init__.py is a bad idea :thinking:

(\cc @aaugustin).

felixxm commented 3 years ago

We could add a warning to the docs that importing django.apps in __init__.py is a bad idea

OK, it's already there:

If your code imports the application registry in an application’s __init__.py, the name apps will clash with the apps submodule. The best practice is to move that code to a submodule and import it. A workaround is to import the registry under a different name:

claudep commented 3 years ago

Thanks a lot!