Opened 11 years ago
Closed 11 years ago
#21188 closed Cleanup/optimization (fixed)
Use DeprecationWarning subclasses for deprecated features
Reported by: | Anssi Kääriäinen | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | adi@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The idea is to introduce new deprecation subclass for each Django version. For example, in current master:
class ToBeRemovedInDjango19Warning(PendingDeprecationWarning): pass class ToBeRemovedInDjango18Warning(DeprecationWarning): pass
By using these warnings we can achieve the following:
- Moving PendingDeprecationWarning to DeprecationWarning after release is single line change.
- It will be possible to filter on Django's deprecation warnings without catching warnings from non-django components.
- It will be harder to miss removal of code. After 1.7 is released just remove ToBeRemovedInDjango18Warning. Any code importing that warning will immediately fail. Also, grepping might be a little easier.
- After release, currently introducing a new PendingDeprecationWarning before PendingDeprecationWarnings are renamed to DeprecationWarnings (as part of release procedure) the new deprecatin will likely be moved to DeprecationWarning. But if a ToBeRemovedInDjango20Warning is added it will be really hard to accidentally move that one to DeprecationWarning.
- It will be technically easy to provide for longer than two releases deprecation periods (if we ever want this).
Drawbacks:
- If we use automatic filtering for deprecations, then code currently doing:
warning.simplefilter("always", DeprecationWarning)
will no longer catch Django's deprecation warnings (as we added an entry for
warning.simplefilter("default" ToBeRemovedInDjango18Warning)
which matches before DeprecationWarning.
I tried this approach in: https://github.com/akaariai/django/compare/deprecation_subclasses#L3R18 - The patch is totally incomplete so I will not mark "has patch".
Change History (9)
comment:1 by , 11 years ago
Description: | modified (diff) |
---|
comment:2 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 11 years ago
To increase perceived urgency, I suggest using DeprecatedInDjango16
for features that go away in Django 1.8, etc.
comment:4 by , 11 years ago
If using DeprecatedInDjango16 instead of ToBeRemovedInDjango18 then it will be impossible to use shorter or longer deprecation periods. If I am not mistaken shorter periods have been used sometimes in the past.
ToBeRemovedInDjango18Warning isn't a good name, so something better is welcome...
comment:5 by , 11 years ago
how about DeprecationInDjango18Warning
. That keeps the same context that is used now and just adds a time when it will happen.
comment:6 by , 11 years ago
Cc: | added |
---|
comment:7 by , 11 years ago
Has patch: | set |
---|
This pull request should be almost complete: https://github.com/django/django/pull/2376
comment:8 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Reviewed the PR by moving warnings subclasses from django.core.exceptions
to django.utils.deprecation
and it seems to be completed.
I ran the full test suite against the {Py2, Py3} X {SQLite3, PostgreSQL} matrix with python -Wonce
and didn't get any failures or warnings (except the already existing wizard ones on Py3).
comment:9 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fancy!