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 Collin Anderson, 10 years ago

Seems to me, because the check framework is still very new, could we deprecate and rename the check method for models?

comment:2 by Tim Graham, 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 Carl Meyer, 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 Tim Graham, 10 years ago

Resolution: wontfix
Status: newclosed

Closing for now as I don't think bumping it to "someday/maybe" in the meantime offers much of an advantage.

Note: See TracTickets for help on using tickets.
Back to Top