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.
Follow-up to #1760786: Move entity system "back" into a Drupal\Core component
Problem
- Every module that has configurable thingies needs to depend on config.module.
Goal
- Untangle ConfigEntity from Config module.
Proposed solution
-
Move the
ConfigEntity*
classes intoDrupal\Core\ConfigEntity\
. -
Remove dependencies on config.module from existing modules in HEAD providing Configurables (config_test.module).
Comment | File | Size | Author |
---|---|---|---|
#15 | config.core_.15.patch | 5.6 KB | sun |
#15 | interdiff.txt | 4.26 KB | sun |
#7 | config-1785974-4.patch | 5.58 KB | tim.plunkett |
#4 | views-1751358-12.patch | 47.8 KB | tim.plunkett |
#4 | interdiff.txt | 1.01 KB | tim.plunkett |
Comments
Comment #1
alexpottGonna take a crack at this tonight.
Comment #2
alexpottMoves the ConfigEntity classes to
Drupal\Core\ConfigEntity\
Comment #4
tim.plunkettBecause config_test.module used to depend on config.module, the entity_info cache would be cleared just at the right time to prevent the test from blowing up.
Now that it has no dependencies, ConfigInstallTest is exposing the bug, which is shown in the interdiff.
Also, rerolled with renames on so that the patch is easier to read.
Comment #5
effulgentsia CreditAttribution: effulgentsia commentedWrong patch posted to #4?
Comment #7
tim.plunkettWhoops! That's what I get for working on two issues at once.
Comment #8
sunLooks ready to fly for me if bot comes green.
Last call for bikeshed: Do we wanna go with Core\ConfigEntity\ ? ;)
(Backstory: Neither @alexpott nor me knew whether it belongs into Entity or Config, so I just simply put ConfigEntity into the summary :P)
Comment #9
tim.plunkettI was curious about that as I rerolled it, it seems really ugly as-is.
Since we have Drupal\Core\Entity\ContentEntityInterface, it would make sense to have it all live under Drupal\Core\Entity. And we could clean up a good amount of
use
statements too...Comment #10
gddCore\ConfigEntity makes sense to me. If it needs to live somewhere else then personally I think Config\ is a better fit than Entity\. It is really a member of the Config subsystem, not the Entity subsystem (even though it is an implementation of Entity.)
Comment #11
effulgentsia CreditAttribution: effulgentsia commentedMy vote would be to put it into Drupal\Core\Config, which may require renaming ConfigStorageController to ConfigEntityStorageController to disambiguate ConfigEntity storage from Config storage. But I don't feel strongly enough about this to hold up the issue for it. I agree with #10 that ConfigEntity is no more part of the Entity system than Node is.
Comment #13
alexpott#7: config-1785974-4.patch queued for re-testing.
Comment #14
tim.plunkettOh, hmm, after reading the other replies I agree.
I was thinking solely about ContentEntityInterface, which is probably also misplaced...
Comment #15
sunLet's do it in a true component way.
Moved Core\ConfigEntity\* into Core\Config\Entity\*.
Comment #16
alexpottMakes sense 1+
Comment #17
andypost+1 it removes a lot of dependencies
Comment #18
catch#15: config.core_.15.patch queued for re-testing.
Comment #19
catchCommitted/pushed to 8.x, thanks!