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.
As the title says.
Comment | File | Size | Author |
---|---|---|---|
#15 | interdiff.txt | 1.83 KB | claudiu.cristea |
#15 | plugin-discovery-recursion-2103155-15.patch | 3.83 KB | claudiu.cristea |
Comments
Comment #1
tim.plunkettTagging.
Comment #2
EclipseGc CreditAttribution: EclipseGc commentedShould have written it this way myself the first time through. ++ and RTBC if it passes.
Eclipse
Comment #4
claudiu.cristeaHere's a patch that introduces a max depth argument. We don't want to leave the discovery dig to much. Passing -1 as $max_depth still permits unlimited level descending (that is not recommended). The default is 0 meaning the actual behavior.
Introducing this argument we don't have to be afraid about performance, so I'm removing the tag.
I'm taking this issue just to keep track. Forgot the interdiff bit it's easy to read diffs.
Comment #5
claudiu.cristeaAh! Forgot new test plugins :)
Comment #6
claudiu.cristea#2121541: Allow recursion when discovering annotated plugins is marked as duplicate of this.
Comment #7
claudiu.cristeaSmall doc fix.
Comment #8
dawehner7: plugin-discovery-recursion-2103155-7.patch queued for re-testing.
Comment #9
claudiu.cristeaThe only thing I'm not very sure with this patch is if this protected property needs to be added also to
DefaultPluginManager
, as we pass the$max_depth
in the constructor. I'm tempted to say "No", because we need the max depth only to be passed to discovery component.Comment #10
chx CreditAttribution: chx commentedI do not think adding complexity in the form of maximum depth is necessary. Nor do I think any performance concerns can be possibly valid when the annotated discovery is insanely slow already. It's cached and if you remove the caching decorator ( I tried for debug ) you'll think HEAD is broken it takes so long to do anything that you'll think it's not happening.
Comment #11
claudiu.cristea@chx,
Max depth is a functional add not necessary a performance improvement. It was only a side reply to #1. I think we need to limit descending level.
Comment #12
claudiu.cristeaOK. Here's without max depth.
Comment #14
chx CreditAttribution: chx commentedSo, to clairfy more, plugin directories will, in my opinion never be deep. In fact, most of the time there will be no subdirs even.
Comment #15
claudiu.cristeaThe test had to be adjusted for descending in subdirectories. But this is good as we need to test this new feature too.
Comment #16
tim.plunkettLooks good, works like it says.
Comment #17
chx CreditAttribution: chx commentedThis is a blocker for migrate so elevating to major. I am itching to elevate it to critical though... it's just unnatural not to pick up recursively.
Comment #18
catchCommitted/pushed to 8.x, thanks!