Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
#2005716: Promote EntityType to a domain object introduced the ability for Annotation classes to return a class object to the annotation instead of an array. This is great, but derivatives is hardcoded to expect arrays, so we need to fix that.
Eclipse
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 1.67 KB | dawehner |
#11 | plugin_derivative_object-2168159-11.patch | 37.24 KB | dawehner |
#8 | interdiff.txt | 2.82 KB | dawehner |
#8 | plugin_derivative_object-2168159-8.patch | 37.18 KB | dawehner |
#3 | plugin_derivative-2168159-3.patch | 35.09 KB | dawehner |
Comments
Comment #1
dawehnerThat was a bit more tricky than expected.
Comment #2
EclipseGc CreditAttribution: EclipseGc commentedif ((is_array($base_definition) || ($base_definition = (array) $base_definition)) && (isset($base_definition['derivative']) && $class = $base_definition['derivative'])) {
This is my only qualm with any of this, I really think we want a method for derivative retrieval if it's a class based definition. Casting the annotation class to an array just doesn't jive for me after going to this much trouble to make it a class, so let's get a real method for this.
Eclipse
Comment #3
dawehnerRerole.
Comment #4
dawehner3: plugin_derivative-2168159-3.patch queued for re-testing.
Comment #7
damiankloip CreditAttribution: damiankloip commentedCould we test the data in the definitions too? Just to make super sure.
Comment #8
dawehnerSure.
Comment #9
damiankloip CreditAttribution: damiankloip commentedOops.
Array items on separate lines I think. Makes it much more readable.
Otherwise, this looks good to go.
Comment #10
BerdirThe new optional provider check in DefaultPluginManager is affected by this too, see #2039435: Convert EntityManager to extend DefaultPluginManager. Do we want to fix this here as well, or somewhere else?
Comment #11
dawehnerCan't we fix that in the other issue?
Comment #12
tim.plunkettOh yay! Glad to see this done.
Comment #13
alexpottCommitted 48025f9 and pushed to 8.x. Thanks!