Spin-off from #1867856: Use annotation discovery for data type plugins. We found that the SchemaDiscovery (which we removed in the aforementioned issue and merged it into the TypedConfigManager) should not run the discovery mechanisms in the constructor but instead just invoke that in getDefinitions() when requested. Patch incoming (including some other minor improvements).
Comment | File | Size | Author |
---|---|---|---|
#9 | interdiff.txt | 577 bytes | fubhy |
#9 | 2030859-9.patch | 5.16 KB | fubhy |
#7 | 2030859-7.patch | 5.4 KB | fubhy |
#4 | 2030859-4.patch | 5.33 KB | fubhy |
#1 | 2030859-1-for-review.patch | 5.33 KB | fubhy |
Comments
Comment #1
fubhy CreditAttribution: fubhy commentedComment #2
Jose Reyero CreditAttribution: Jose Reyero commentedAgree with the need to fix SchemaDiscovery, but this patch looks a bit too long to me...? (It looks like it includes too #1867856: Use annotation discovery for data type plugins )
Comment #3
fubhy CreditAttribution: fubhy commentedYes, @see the "for-review" patch ;).
Comment #4
fubhy CreditAttribution: fubhy commentedRe-uploading that patch for testbot :)
Comment #5
Gábor HojtsyLooks good to me.
Comment #6
fubhy CreditAttribution: fubhy commentedActually, that should be String::format() :(
Comment #7
fubhy CreditAttribution: fubhy commentedComment #8
Jose Reyero CreditAttribution: Jose Reyero commentedYes, this one looks good to me, only one minor issue:
Could we keep this last line so we don't need to reprocess the same definition twice and it gets kind of cached, mostly for performance reasons?
Comment #9
fubhy CreditAttribution: fubhy commentedSure... Dunno how this affects the tests or why I even removed it. But we'll see! :)
Comment #10
Jose Reyero CreditAttribution: Jose Reyero commentedLooks good and green :-), thanks, I think this is ready.
(and required for this one to work: #1966538: Translation is not updated for configuration, when the config changes)
Comment #11
Gábor HojtsyTag as blocker being a dependency.
Comment #12
alexpottCommitted d7bbf9f and pushed to 8.x. Thanks!