Deprecate StorageComparerInterface

Created on 20 December 2023, 4 months ago
Updated 3 April 2024, 10 days ago

Problem/Motivation

From ๐Ÿ“Œ make it easier to instantiate ConfigImporter Needs work :

> 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

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Configuration entityย  โ†’

Last updated 1 day ago

Created by

๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

Live updates comments and jobs are added and updated live.
  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joachim
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡จ๐Ÿ‡ญSwitzerland bircher ๐Ÿ‡จ๐Ÿ‡ฟ

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

  • First commit to issue fork.
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada Chris_Tomasso

    Hello,
    I will work on this one!

  • Pipeline finished with Failed
    about 1 month ago
    Total: 172s
    #110875
  • Pipeline finished with Failed
    about 1 month ago
    Total: 195s
    #110880
  • Pipeline finished with Failed
    about 1 month ago
    Total: 170s
    #110895
  • Pipeline finished with Failed
    about 1 month ago
    #110897
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    Binoli Lalani โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    11 days ago
    Total: 481s
    #135392
  • Pipeline finished with Success
    11 days ago
    Total: 632s
    #135419
  • Status changed to Needs review 11 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    Hello,

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

    Thanks!

  • Status changed to Needs work 11 days ago
  • ๐Ÿ‡จ๐Ÿ‡ญSwitzerland bircher ๐Ÿ‡จ๐Ÿ‡ฟ

    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.

  • Pipeline finished with Failed
    10 days ago
    Total: 172s
    #136264
  • Pipeline finished with Success
    10 days ago
    Total: 604s
    #136283
  • Status changed to Needs review 10 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    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 ๐Ÿ“Œ Deprecate TrustedCallbackInterface in favour of TrustedCallback attribute Needs work , https://www.drupal.org/project/drupal/issues/2266817 ๐Ÿ“Œ Deprecate empty AccessInterface and remove usages Needs work

    I updated the code in the MR. Please review.

    Thanks!

  • Status changed to Needs work 10 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    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.

Production build https://api.contrib.social 0.62.1