Opened 10 years ago
Closed 10 years ago
#23671 closed Bug (wontfix)
Model Field named "check" clashes with check framework.
Reported by: | Collin Anderson | Owned by: | nobody |
---|---|---|---|
Component: | Core (System checks) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
#23615 dealed with this issue in 1.7, but how should we handle it in 1.8?
Change History (4)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
We already documented the backwards incompatibility in the 1.7 release notes, "As part of the System check framework, fields, models, and model managers all implement a check() method that is registered with the check framework. If you have an existing method called check() on one of these objects, you will need to rename it." I'm not sure changing this again is going to be worthwhile.
comment:3 by , 10 years ago
I agree with Tim. Once we've introduced and released a backwards compatibility, there is no benefit to reversing it in the next release. Almost everyone upgrades version-by-version, so changing the check()
method to something else now is just doubling the pain, not reducing it.
I think we should wait and see if in practice the 1.7 compromise (fields named check
are only disallowed if they set a descriptor on the model class) is painful and confusing. If it is, then in 1.8 we should consider disallowing all fields named check
. If it isn't, maybe we leave well enough alone.
comment:4 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Closing for now as I don't think bumping it to "someday/maybe" in the meantime offers much of an advantage.
Seems to me, because the check framework is still very new, could we deprecate and rename the check method for models?