Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
entity system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
13 Feb 2024 at 03:09 UTC
Updated:
9 May 2024 at 20:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
mstrelan commentedPostponed on #3420987: Convert DataType plugin discovery to attributes
Comment #3
kim.pepper#3420987: Convert DataType plugin discovery to attributes is in so this is unblocked.
Comment #5
kim.pepperComment #6
mstrelan commentedCan we update the docs for hook_field_info_alter in field.api.php here too? There are references to the annotation class but we should update to the attribute class.
Comment #7
kim.pepper#6 Done.
I was also questioning how these worked with
Annotation\FieldTypeas they weren't defined anywhere I can see:Comment #8
mstrelan commented#7 I think we can remove those. They are the additional keys mentioned at the top of the attribute/annotation class. Since the attribute values get converted to an array they don't need to be defined.
Comment #9
mstrelan commentedActually I think if we need to remove them then we need an alter hook to add the values in.
Comment #10
berdirWhat I'm doing in the entity type issue is add an explicit additional key where you can put it arbitrary settings.
But some of these we probably should define if they're part of the official API.
serialized property names is part of a security issue that went in a while ago, column_groups is afaik mostly used for content _translation, not sure about config dependencies. It's used in \Drupal\Tests\field\Kernel\TestItemWithDependenciesTest, would probably need to check the issue that added this to see what the real use cases are.
Comment #11
kim.pepper@Berdir #10 i had a look at #3396166: Convert entity type discovery to PHP attributes and doesn't look this you are doing this anymore?
I have left the properties in for now.
Re: config_dependencies I found this in the docs for
\Drupal\Core\Config\Entity\ConfigDependencyManagerComment #12
smustgrave commentedApplied the MR searched for @FieldType all instances have been replaced
Verified $deriver is there (something had to check from previous attribute tickets)
Tests are all green, showing nothing broke.
Believe this is good
Comment #13
alexpottComment #14
kim.pepperAddressed feedback so back to NR
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #17
quietone commentedComment #18
smustgrave commentedAll feedback appears to be addressed here.
Comment #19
alexpottCommitted and pushed cf09596544 to 11.x and 4afc682d90 to 10.3.x. Thanks!