#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.

Files: 
CommentFileSizeAuthor
#9 config.classes.9.patch13.75 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,577 pass(es). View
#6 config.classes.6.patch13.72 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,559 pass(es). View
#5 config.classes.5.am_.patch18.69 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config.classes.5.am_.patch. Unable to apply patch. See the log in the details link for more information. View
#5 config.classes.5.patch13.78 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,553 pass(es). View
#2 config.classes.2.patch18.69 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config.classes.2.patch. Unable to apply patch. See the log in the details link for more information. View
#1 config.classes.1.patch8.99 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,571 pass(es). View

Comments

sun’s picture

Status: Active » Needs review
FileSize
8.99 KB
PASSED: [[SimpleTest]]: [MySQL] 36,571 pass(es). View
sun’s picture

FileSize
18.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config.classes.2.patch. Unable to apply patch. See the log in the details link for more information. View

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

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

FileSize
13.78 KB
PASSED: [[SimpleTest]]: [MySQL] 36,553 pass(es). View
18.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config.classes.5.am_.patch. Unable to apply patch. See the log in the details link for more information. View

Odd. #2 should apply cleanly. Trying again.

sun’s picture

FileSize
13.72 KB
PASSED: [[SimpleTest]]: [MySQL] 36,559 pass(es). View

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

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.

heyrocker’s picture

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

Status: Needs work » Needs review
FileSize
13.75 KB
PASSED: [[SimpleTest]]: [MySQL] 36,577 pass(es). View

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

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

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

heyrocker’s picture

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

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

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

Ayup

sun’s picture

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

catch’s picture

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.