Problem/Motivation

From #3123491: make it easier to instantiate ConfigImporter:

> I want to avoid swapability at all costs here. Supporting alternate implementations of the ConfigImporter and StorageComparer makes me feel very uneasy. [SNIP]
> I wish \Drupal\Core\Config\StorageComparerInterface did not exist. Perhaps we can move all the docs to the StorageComparer and change the typehint on \Drupal\Core\Config\ConfigImporter::__construct() to StorageComparer|StorageComparerInterface and deprecate the interface.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3410037

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joachim created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: deprecate StorageComparerInterface » Deprecate StorageComparerInterface
Issue tags: +@deprecated
bircher’s picture

bircher
Location 🇨🇿
commented

Ah excellent! We discussed this on slack the other day and concluded that it should never have been an interface.

Chris_Tomasso made their first commit to this issue’s fork.

chris_tomasso’s picture

chris_tomasso commented

Hello,
I will work on this one!

Chris_Tomasso opened merge request !6902

Binoli Lalani made their first commit to this issue’s fork.

binoli lalani’s picture

binoli lalani commented
Status: Active » Needs review

Hello,

I fixed the Phpunit testcases error. Please review the MR.

Thanks!

bircher’s picture

bircher
Location 🇨🇿
commented
Status: Needs review » Needs work

I think that this is not correct.
Because with this MR the comparer doesn't implement the interface this will break custom or contrib code which typehints the interface but then passes in a normal comparer. Also this MR does not add the deprecated tag on the interface.

I don't know of the top of my head what other interfaces have been deprecated but looking for one and following the example would be good.

binoli lalani’s picture

binoli lalani commented
Status: Needs work » Needs review

Hello,

Thank you for reviewing the MR.

I added a deprecated tag in the interface and found 2 issues where an interface is deprecated and it's removed from the class where it implements.

Reference issues: https://www.drupal.org/project/drupal/issues/3354584, https://www.drupal.org/project/drupal/issues/2266817

I updated the code in the MR. Please review.

Thanks!

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs subsystem maintainer review

Will need a change record and that's what deprecation link should point to

Probably need a deprecation test too

May want to see if we are good to deprecate though.

chris_tomasso’s picture

chris_tomasso commented

Hello,

I've removed an unnecessary @trigger_error that I didn't believe was required.
I hope this change aligns with your expectations.

You can review the changes here: Merge Request #6902.

Additionally, I’ve included the test and change record as requested by smustgrave.

Let me know if there's anything.

karimb’s picture

karimb commented
Status: Needs work » Needs review
joachim’s picture

joachim commented
Status: Needs review » Needs work

The version needs updating, but apart from that looks good.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.