Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Sorry, another moving things issue.
This is only used in Drupal\field\Plugin\views\field\Field. So we can move this to a method on that class. I have removed the static caching and replaced it with a property on the class.
Does this method need to be updated/replaced with something else from the field api?
Comment | File | Size | Author |
---|---|---|---|
#26 | vdc-1912504-26.patch | 3.95 KB | dawehner |
#20 | 1912504-20.patch | 887 bytes | amateescu |
#20 | interdiff.txt | 598 bytes | amateescu |
#18 | 1912504-18.patch | 606 bytes | damiankloip |
#15 | 1912504-15.patch | 606 bytes | damiankloip |
Comments
Comment #1
dawehnerTo be honest I never understood why we moved that into the views.module file ... let's figure that out first (in d7 this has been part of the field handler).
So if it's borrowed from field_ui we should put a public API in field_ui, as it's helpful for two different places in core already.
Comment #2
damiankloip CreditAttribution: damiankloip commentedSo atm in D8, this function now exists as field_ui_formatter_options in field_ui.admin.inc.
tbh, I'm not sure why we need to copy it? We have enough code to maintain ;) Shall we just switch to including this file and using this function instead for now? hmmm
Comment #4
dawehnerRerolled.
Comment #5
damiankloip CreditAttribution: damiankloip commented#4: drupal-1912504-4.patch queued for re-testing.
Comment #6
damiankloip CreditAttribution: damiankloip commented#4: drupal-1912504-4.patch queued for re-testing.
Comment #8
damiankloip CreditAttribution: damiankloip commentedrerolled.
Comment #9
dawehnerI'm wondering whether we can replace that by a proper @see
Cool a parameter, what is going on.
Comment #10
damiankloip CreditAttribution: damiankloip commentedLet's fix up the docblock.
Comment #11
dawehnerPerfect!
Comment #12
alexpottCommitted f6b5925 and pushed to 8.x. Thanks!
Comment #13
damiankloip CreditAttribution: damiankloip commentedComment #14
tstoecklerSorry in case I'm just being stupid, but I don't see $options being defined in that function at all. That should be
I guess.
Comment #15
damiankloip CreditAttribution: damiankloip commentedYeah, you are totally right. Although in views we will never need that return value, as we won't get all field options. So Shall we just fix it or remove it? @alexpott, I leave it up to you.
Comment #17
tstoecklerI don't really have an opinion which patch should go in, but the "no-return" one, if we decide to go with it, should also make $field_type a required argument and remove the condition for it.
Comment #18
damiankloip CreditAttribution: damiankloip commentedI think we might as well just return formatterOptions, then this works the same as the field api, even though we don't actually use all field info.
Comment #19
dawehnerI would always go with the return one, because people might expect that behavior.
Comment #20
amateescu CreditAttribution: amateescu commentedThere's a patch at #1982138: Clean out field_ui.admin.inc that provides an identical helper in the formatter plugin manager, where it belongs. Can we please add a @todo to remove this duplicated one?
Comment #21
dawehnerSure.
Comment #23
damiankloip CreditAttribution: damiankloip commented#20: 1912504-20.patch queued for re-testing.
Doesn't look like it actually failed.
Comment #24
dawehnerReRTBC
Comment #25
alexpott#1982138: Clean out field_ui.admin.inc has landed so lets do the removal here...
Comment #26
dawehnerManual testing still worked.
Comment #27
damiankloip CreditAttribution: damiankloip commentedNice, looks good. Plus there is already test coverage for this in the UI. I like removing code we don't really need. This was originally here just so we didn't have to load the field_ui.admin.inc file.
Comment #28
amateescu CreditAttribution: amateescu commentedIf this is really true, I'm speechless..
Anyway, updated the title to tell what's really going on in the latest patch.
Comment #29
damiankloip CreditAttribution: damiankloip commentedOr something like that anyway..
Comment #30
alexpottCommitted 8cc0c63 and pushed to 8.x. Thanks!
Comment #32
xjmComment #33
Chris Matthews CreditAttribution: Chris Matthews as a volunteer and at City of Oaks Design commentedFor more information as to why this issue was moved to the Drupal core project, please see issue #3030347: Plan to clean process issue queue
Comment #34
Chris Matthews CreditAttribution: Chris Matthews as a volunteer and at City of Oaks Design commentedMoving back to the contributed Views issue queue and closing as outdated per https://www.drupal.org/project/views/issues/3030347#comment-13023447