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.
Problem/Motivation
Now that #2010412: [Meta] Change plugin managers to use DefaultPluginManager is done, nothing is really using those two anymore, so we should remove them.
Proposed resolution
Remove the two classes, convert the remaining usage in the test plugin manager.
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#7 | remove-decorators-2271363-7.patch | 26.71 KB | Berdir |
#1 | remove-decorators-2271363-1.patch | 30.22 KB | Berdir |
Comments
Comment #1
BerdirFirst patch, removing a bunch of tests and test code that I think we no longer need because we have enough test coverage through unit tests and integration tests with the actual plugin manager.
Also removed CachedDiscoveryInterface from PluginManagerBase because there's no discovery that implements that anymore.
Comment #2
dawehnerI don't get why we always have to follow the rule: if core does not use it, there is no use for it. Using decorators is a great idea in the first place, because it shows the flexiblity of the system.
Comment #3
BerdirBecause we agreed that those three are a bad pattern, CacheDecorator is the reason why we had to add ProcessDecorator/AlterDecorator so that we can move that inside the cache. And we did that even though EclipseGc and neclimdul were against it.
Which is why we created DefaultPluginManager.
So my reason for removing this is that we should not promote a bad pattern, and it's 700 lines of code less to maintain.
But mostly, I created this issue to start the discussion if we want to do this and/or only parts of this.
Comment #4
EclipseGc CreditAttribution: EclipseGc commentedYeah, consider me ++ to this in general. This is the reason the DefaultPluginManager was created for Drupal's use, and that has fixed a TON of problems. Removing the temptation to perpetuate a pattern we worked hard to exterminate seems ++ to me. Understand I completely agree with dawehner in principle, but in this particular case, I am completely on board with removing this code since it's not a utility whose use we wish to perpetuate.
Eclipse
Comment #5
dawehnerOkay, I am convinced as well.
Comment #7
BerdirLooks like you took too long to get convinced ;)
Here's a re-roll.
Comment #10
dawehnersee above
Comment #11
alexpottWe need to remove https://www.drupal.org/node/1887798 and check any docs that exist on d.o. :)
Comment #12
alexpottCommitted 0fcb80b and pushed to 8.x. Thanks!