Opened 11 years ago
Closed 11 years ago
#23750 closed Cleanup/optimization (fixed)
django.core.checks.register shouldn't be (primarily) a decorator
| Reported by: | Shai Berger | Owned by: | averybigant |
|---|---|---|---|
| Component: | Core (System checks) | Version: | 1.7 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
django.core.checks.register is commonly used as a function, called from AppConfig.ready(), and not as a
decorator; but it is defined as a decorator, so it looks a little funny:
checks.register('models')(check_generic_foreign_keys)
(from the contenttypes AppConfig, for example).
Registering checks only in ready(), rather than at import time, is now the way
to do things, so register() should typically not be used as a decorator; I
think we should change its definition accordingly, to
def register(self, check, *tags)
The above is, essentially, an old message of mine to django-developers; in that thread, you can find Russell agreeing, and Marc adding that the checks documentation needs to change as well. However, since 1.7 was already released, register needs to be changed in a backwards-compatible way -- so that it can be used either as it is currently, or using the more natural style.
Change History (5)
comment:1 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:2 by , 11 years ago
| Has patch: | set |
|---|---|
| Needs tests: | unset |
follow-up: 4 comment:3 by , 11 years ago
| Patch needs improvement: | set |
|---|
I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.
comment:4 by , 11 years ago
| Patch needs improvement: | unset |
|---|
Replying to timgraham:
I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.
new commit pushed.
comment:5 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
pull request: https://github.com/django/django/pull/3473