This is a list to track issues to cleanup Field API procedural functions and replace them with new API. Most issues already have new API in place. Some might not, but in light of doing cleanups, it might be nice to clean those up too.
- #2018319: Remove field_read_field(s)() and field_read_instance(s) in favor of entity_load() and entity_load_multiple_by_properties() : has been green before API freeze, but due to lack of time didn't get a review in a time.
- #2047997: Get rid of field_behaviors_widget(): old leftover from CCK, should simply be removed. Only Drush was a 'big' user of this function for generating files.
- #2047993: Remove current uses of field_info_*_types() / field_info_*_settings(): marked some functions deprecated, but we didn't remove them for now (no issue yet, easy patch)
- #2051721: Get rid of field_get_items(): marked field_get_items deprecated, could just be removed. (no issue yet, easy patch)
- #2019147: Mark functions in field.info.inc deprecated.: marked a lot of functions in field.info.inc deprecated, could just be removed (no issue yet, easy patch)
- #2029835: Deprecate procedural functions which have been moved to the Field class: wants to deprecate/remove remaining functions in field.info.inc - needs work (could remove the other ones from previous)
- #2067079: Remove the Field Language API: TBD, needs feedback from "Entity translation API" folks
These are the ones that given the time we still have are feasible to get done. We still also have the BC layer that needs to be removed too, see #1953408: Remove ArrayAccess BC layer from field config entities. After all this stuff, we'll have a pretty damn modern Field API.
See also #2067127: Reorganize the contents of field.* files for a file reorg to get a clearer view of the current status & contents of field.module
Comment | File | Size | Author |
---|---|---|---|
#11 | field_deprecated-2061107-11.patch | 13.51 KB | yched |
Comments
Comment #1
swentel CreditAttribution: swentel commentedtagging
Comment #2
xjmSo there are basically three parts to this for each of these issues:
#3 is the part that will need to be evaluated by a core maintainer to consider whether it can be allowed after API freeze. My thought is that there is a lot of value in removing the D7 API.
field_whatever_()
on api.d.o to find what they need.All that said, @swentel pointed out (and I agree) that these issues are less important than solving critical issues like:
...so if we need to make tough calls about what to prioritize to get the Field API done, these issues will have to take a back seat to those, and we should avoid disrupting those patches where possible.
Comment #2.0
xjmUpdated issue summary.
Comment #4
xjmTag monster--
Comment #5
yched CreditAttribution: yched commentedThere are also the field.multilingual.inc functions, on which the way forward is unclear to me :-)
Opened #2067079: Remove the Field Language API, added it to the issue summary.
Comment #5.0
yched CreditAttribution: yched commentedUpdated issue summary.
Comment #6
yched CreditAttribution: yched commentedAlso, posted a patch in #2067127: Reorganize the contents of field.* files, to get a clearer view of the current status & content of field.module.
Comment #6.0
yched CreditAttribution: yched commentedAdded #2067079: Remove the Field Language API
Comment #7
xjmFor this issue, @Dries suggested that things that are already marked
@deprecated
and simply wrap the new implementations can be removed. For those that don't yet have a replacement, we can add the replacement, but should probably retain the BC wrappers. We also agreed that these cleanups are less important than #1953408: Remove ArrayAccess BC layer from field config entities and other critical field API patches, and so we might do best to resolve those issues first to avoid disruption from these changes.Comment #8
yched CreditAttribution: yched commentedAgreed that #1953408: Remove ArrayAccess BC layer from field config entities is more important.
Would be good if we could get #2067127: Reorganize the contents of field.* files in soonish, would make those things easier to sort out.
Comment #8.0
yched CreditAttribution: yched commentedAdded #2067127: Reorganize the contents of field.* files
Comment #9
webchickA case has been made by several folks that keeping these old functions around represents a big enough learnability challenge for the new Entity/Field API that we should most likely clean this up prior to beta.
I'd like to do this as one patch, however, to remove the entirety of field.deprecated.inc, once those functions all actually have OO replacements that are used throughout core. That avoids overwhelming the RTBC queue with a bunch of issues like #2155697: Remove a couple simple deprecated functions (and their associated review/re-roll cycle hell), and also keeps focus on the things that matter (making sure those new OO subsystems exist and are complete and functional), with this being a nice cherry on top of the sundae at the end once we've done all the hard work.
So: tagging, postponing, un-metaing.
Comment #10
BerdirAgreed that an issue like #2155697: Remove a couple simple deprecated functions doesn't make much sense, but what about something like #2157153: Remove field_attach_preprocess()? It removes usage of that function and removes it as well.
Field API includes a ton of (deprecated) functions over a wide range of things, language related, preprocessing, widget/formatter handling, accessing field values and so on. Having a separate issue to remove the usage and mark it deprecated, then remove it here seems actually more work to me, removing both usage and the functions in here IMHO makes harder to review and violates http://webchick.net/please-stop-eating-baby-kittens.
Some of the "deprecated" functions don't even have a replacement yet, there again makes it more sense to me to remove that issue in the issue that provides the replacement, e.g. to prove that nothing is remaining that's still using it.
Apart from the mentioned issue which randomly removes some functions that happen to be not used anymore and leaves others, this seems like a different scenario than the hasPermission() or unused variables meta issues to me.
Comment #11
yched CreditAttribution: yched commented@webchick:
Unfortunately, it's not that simple :-/
Here's a rundown on where we stand wrt field.deprecated.inc:
The following have replacements and are not used in core anymore. They can be removed any time.
For the rest:
Field registry :
The final API depends on the outcome of #2116363: Unified repository of field definitions (cache + API), so there's little point in forcing people to move now, the replacements we would point them to might be obsolete in a couple weeks.
Field attach * :
@deprecated is a lie, we currently don't have replacements for those, and current core still calls them. That's what #2095195: Remove deprecated field_attach_form_*() is about.
- Then, as @Berdir points, there's field_attach_preprocess(), which is *not* marked as deprecated and is still called, but is actually totally useless with Twig.
#2157153: Remove field_attach_preprocess() simply removes it, without needing any other code or test adjustment.
----------------
In short: the functions we can't remove right now have their own critical but thorny issues, I don't think it's reasonable to wait on them. I'd plea that we remove what can be removed now - honestly, getting them out of our mental plate would help.
Attached patch takes care of the ones listed at the top, and #2157153: Remove field_attach_preprocess() is about field_attach_preprocess().
Comment #12
xjmThat's very helpful information. Thanks a ton @yched.
Comment #13
xjmQuick followup: So the first group could be removed in one patch here, the second group maybe deserves its own postponed issue, and the third group probably needs one or multiple issues blocked on #2095195: Remove deprecated field_attach_form_*()?
Can we file issues for the second and third things (one for group B, one for group C) if they don't exist yet, tagged as beta blockers and postponed on the appropriate blocking issues?
Comment #14
xjmAlso, @webchick already approved ripping out all these deprecated wrappers (not sure how the tag didn't end up on the issue when we discussed it last week).
Comment #15
xjmOh, or does #2095195: Remove deprecated field_attach_form_*() already remove all those functions in the third group (or could/should it)?
Comment #16
chx CreditAttribution: chx commentedThis is lovely!
Comment #17
webchickOk. I think that split makes sense. Thanks for the background.
Committed and pushed to 8.x.
We'll need a change notice if we don't have one already.
Comment #18
yched CreditAttribution: yched commentedThanks folks :-) Will look into the change records asap.
@xjm #13 / #15
#2095195: Remove deprecated field_attach_form_*() currently removes core calls but keeps the existing functions, as per #9.
Also, current in-progress patch only deals with field_attach_form_*() and is still debated, so if things don't progress fast enough I'm considering maybe splitting a separate issue for field_attach_*_view()...
Comment #19
xjmFiled #2167167: Remove field_info_*() and suggested a re-scope for #2095195: Remove deprecated field_attach_form_*() (since it was already removing all usage, it's completely within scope and preferable to remove the functions themselves there as well, since we've committed to removing them before beta).
Comment #20
yched CreditAttribution: yched commentedre: change notice
So far we updated the existing change notices for the respective topics with further changes.
I.e. :
- field_info_field_types() / field_info_field_settings() / field_info_instance_settings
would go in Field types are now plugins
- field_info_widget_types() / field_info_widget_settings()
would go in Field widgets are now plugins
- field_info_formatter_types() / field_info_formatter_settings()
would go in Field formatters are now plugins
- field_read_*()
would go in Field and field instance are now configuration entities
...
Do we go for this, or do you want separate change records ?
Comment #21
xjmYep, I think updating the existing change records makes sense. Thanks!
Comment #22
yched CreditAttribution: yched commentedUpdated the change notices:
field_info_field_types() / field_info_field_settings() / field_info_instance_settings():
https://drupal.org/node/2064123/revisions/view/6814725/6817957
field_info_widget_types() / field_info_widget_settings()
https://drupal.org/node/1796000/revisions/view/2889377/6817965
field_info_formatter_types() / field_info_formatter_settings()
https://drupal.org/node/1805846/revisions/view/6814731/6817973
field_read_*()
https://drupal.org/node/2012896/revisions/view/2864639/6817981
field_get_items()
https://drupal.org/node/2055615/revisions/view/2786839/6818019
field_get_default_value()
https://drupal.org/node/2070839/revisions/view/2811775/6818009
field_access()
was already covered by https://drupal.org/node/2101719
Comment #23
yched CreditAttribution: yched commentedSide note:
- Updated $entity_type argument removed from Field API functions and hooks dealing with a single $entity and removed the functions / hooks that have disappeared since then...
https://drupal.org/node/1882428/revisions/view/2549066/6818073
- field_get_items() returns an empty array, rather than false, when no items exist is now irrelevant and should be removed, but it seems I don't have the proper permissions to do this.
Comment #24
xjmI deleted the second one. Thanks!