Problem/Motivation

The StorageComparer takes as arguments a source and a target storage and can then be used to figure out what the differences are between them. However the constructor of the StorageComparer also takes a ConfigManagerInterface object which it never uses for anything because the comparison can be done by just looking at the two storages.

Proposed resolution

Remove the dependency on ConfigManagerInterface and leave an optional argument in the constructor so that it remains backwards compatible.

Remaining tasks

write patch
review
commit

User interface changes

none

API changes

omitting unnecessary argument for the StorageComparer.

Data model changes

none

CommentFileSizeAuthor
#4 interdiff-2993271-2-4.txt1.86 KBbircher
#4 2993271-4.patch16.29 KBbircher
#2 2993271-2.patch16.29 KBbircher

Comments

bircher created an issue. See original summary.

bircher’s picture

bircher
Location 🇨🇿
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2993271-2.patch16.29 KB

Attached is a patch.

upchuk’s picture

upchuk commented
Status: Needs review » Needs work

This is a good idea and looks good. Just a minor typo that I found:

  1. +++ b/core/lib/Drupal/Core/Config/StorageComparer.php
@@ -111,8 +104,11 @@ public function __construct(StorageInterface $source_storage, StorageInterface $
+      @trigger_error('The storage compared does not need a config manager. The parameter is deprecated since version 8.7.0 and will be removed in 9.0.0. Omit the third parameter. See https://www.drupal.org/node/2993271.', E_USER_DEPRECATED);

    Minor typo. Should be storage "comparer" 

  2. +++ b/core/tests/Drupal/Tests/Core/Config/StorageComparerTest.php
@@ -244,4 +238,13 @@ public function testCreateChangelistUpdate() {
+   * @expectedDeprecation The storage compared does not need a config manager. The parameter is deprecated since version 8.7.0 and will be removed in 9.0.0. Omit the third parameter. See https://www.drupal.org/node/2993271.

    Minor typo. Should be storage "comparer"

bircher’s picture

bircher
Location 🇨🇿
commented
Status: Needs work » Needs review
StatusFileSize
new 2993271-4.patch16.29 KB
new interdiff-2993271-2-4.txt1.86 KB

thanks for the review.

upchuk’s picture

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

Looks, good. Thanks!

catch’s picture

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

Committed 619b274 and pushed to 8.7.x. Thanks!

  • catch committed 619b274 on 8.7.x 
    Issue #2993271 by bircher, Upchuk: Remove unused ConfigManager...

Status: Fixed » Closed (fixed)

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