#1444620: Remove file signing from configuration system removed the file signing, but didn't remove the "Verified" from Drupal\Config class names.

Also, the class names could use some heavy clean-up.

CommentFileSizeAuthor
#9 config.classes.9.patch13.75 KBsun
#6 config.classes.6.patch13.72 KBsun
#5 config.classes.5.am_.patch18.69 KBsun
#5 config.classes.5.patch13.78 KBsun
#2 config.classes.2.patch18.69 KBsun
#1 config.classes.1.patch8.99 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Needs review
StatusFileSize
new config.classes.1.patch8.99 KB
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new config.classes.2.patch18.69 KB

Adjusted docs and renamed DrupalVerifiedStorageSQL into DatabaseStorage.

Status: Needs review » Needs work

The last submitted patch, config.classes.2.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review

FWIW, with this patch, core\lib\Drupal\Core\Config> dir /B yields:

AbstractStorage.php
ConfigException.php
DatabaseStorage.php
DrupalConfig.php
FileStorage.php
FileStorageException.php
FileStorageReadException.php
StorageInterface.php
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new config.classes.5.patch13.78 KB
new config.classes.5.am_.patch18.69 KB

Odd. #2 should apply cleanly. Trying again.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new config.classes.6.patch13.72 KB

Briefly discussed the "Abstract" prefix in IRC. Removed it, as it's not strictly necessary and lines up Storage.php with StorageInterface.php:

ConfigException.php
DatabaseStorage.php
DrupalConfig.php
FileStorage.php
FileStorageException.php
FileStorageReadException.php
Storage.php
StorageInterface.php
yched’s picture

yched commented

Denoting the abstract nature of the class within the class name is precious IMO.
Shouldn't be a blocker for this patch, but I wish we settled some convention regarding those "abstract" classes - opened #1567920: Naming standard for abstract/base classes about that.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Needs review » Needs work

I don't have a strong feeling on this but based on the results of #1567920: Naming standard for abstract/base classes it appears that a standard has been essentially decided, so I'm putting this to CNW since it will undoubtedly need a reroll.

Otherwise I'm liking this a lot. Thanks sun.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new config.classes.9.patch13.75 KB

Renamed Storage to StorageBase.

FWIW, this change is also available as a pull-able branch in cmi/config-classes-1567812-sun

(At some point, I'd like to have some clarity on what patches will be merged and which not, 'cos creating and maintaining feature branches involves a certain overhead.)

Status: Needs review » Needs work
Issue tags: -API clean-up, -Configuration system

The last submitted patch, config.classes.9.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
Issue tags: +API clean-up, +Configuration system

#9: config.classes.9.patch queued for re-testing.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented

I haven't been maintaining feature branches for anything of late, although I was thinking about it for the rearchitecture issue since it is a more colossal change. I agree it is all pretty loose at this point and it would be nice to have some standards.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Alright, "Drupal installation failed" was a testbot hiccup. Ready to fly?

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

Ayup

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

#9: config.classes.9.patch queued for re-testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

We're over thresholds but internal stuff in cmi can't possibly conflict with any critical issues so I've gone ahead and committed this one.

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