While working on #2188613: Rename EntityStorageController to EntityStorage, I noticed that a lot of places in config refer to the config storage as "storage controller". It's only comments, no actual code, but it should still be cleaned up, this seems confusing.

Just replace "storage controller" with "storage", but wait for the issue above to land.

CommentFileSizeAuthor
#14 drupal8-2227187-14.patch15.06 KBJalandhar
#12 drupal-2227187-12.patch16.06 KBcs_shadow
#7 interdiff.txt1.58 KBozin
#6 interdiff.txt1.47 KBozin
#5 issue-2227187-5.patch10.7 KBozin
#2 drupal-2227187-replace-storage-controller-2.patch12.15 KBmandar.harkare

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Step refering to config storage as storage controller » Stop refering to config storage as storage controller

Ups.

mandar.harkare’s picture

mandar.harkare commented
Status: Active » Needs review
StatusFileSize
new drupal-2227187-replace-storage-controller-2.patch12.15 KB

Not sure I made the changes in the right places or not.

Cheers.

Status: Needs review » Needs work

The last submitted patch, 2: drupal-2227187-replace-storage-controller-2.patch, failed testing.

jkingsnorth’s picture

jkingsnorth commented

Not sure the new comments make a whole lot of sense, might need more than just a find-replace. ie:

'A configuration storage to be cached.'
'Constructs a new FileStorage.'

Some of the find-and-replaced instances do read OK though. But this needs work.

ozin’s picture

ozin
Primary language Ukrainian
commented
Status: Needs work » Needs review
StatusFileSize
new issue-2227187-5.patch10.7 KB

This is a re-roll.

ozin’s picture

ozin
Primary language Ukrainian
commented
StatusFileSize
new interdiff.txt1.47 KB

Interdiff for #5 comment.

ozin’s picture

ozin
Primary language Ukrainian
commented
StatusFileSize
new interdiff.txt1.58 KB

Sorry, here is correct interdiff for #5.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Issue tags: +LembergSprint

Adding tag.

ribel’s picture

ribel
Primary language Ukrainian
Location 🇺🇦Lviv
commented
Status: Needs review » Reviewed & tested by the community

Good!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
 *   callers like system_list() to force-disable a possible configuration
 *   storage controller cache or some other way to circumvent it/take it over.

module.inc

      // Renaming a config object needs to cater for:
      // - Storage controller needs to access the original object.

ConfigEntityStorage.php

Defines the file storage controller for metadata files.

SchemaStorage.php

Drupal\config\Tests\Storage\DatabaseStorageTest has 3 uses of controller
Drupal\config\Tests\Storage\FileStorageTest has 3 uses of controller

 * Storage controller for migration entities.

MigrationStorage.php (there are 2 of these)

   * The config storage controller.

ConfigEntityStorageTest.php

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

The module.inc @todo is being removed somewhere, although I'm not sure where exactly. The theme critical probably?

Looks like 3 uses are related to entities, but yeah, let's clean that up as well.

cs_shadow’s picture

cs_shadow commented
Status: Needs work » Needs review
StatusFileSize
new drupal-2227187-12.patch16.06 KB

Changing patch in #5 according to changes mentioned in #10.

Status: Needs review » Needs work

The last submitted patch, 12: drupal-2227187-12.patch, failed testing.

Jalandhar’s picture

Jalandhar commented
Status: Needs work » Needs review
StatusFileSize
new drupal8-2227187-14.patch15.06 KB

Updating patch with reroll. Please review it.

filijonka’s picture

filijonka commented
Status: Needs review » Reviewed & tested by the community

this looks good to me, the suggestions in #10 is included/done.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Committed 9a878fb and pushed to 8.x. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

  • Commit 9a878fb on 8.x by alexpott: 
    Issue #2227187 by Mykhailo Gurei, Jalandhar, cs_shadow, mandar.harkare...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.