While trying doing a config import with Drush I got confused by the mixing of the config_comparer and storage_comparer.

Let's call them the same. Latest patch choose for storage_comparer as that resembles mostly to its class.

Original code from #1890784: Refactor configuration import and sync functions

CommentFileSizeAuthor
#7 core-config_comparer-storage_comparer-2012566-7.patch3.4 KBLuka Zitnik
#2 core-config_comparer-storage_comparer-2012566-2.patch3.29 KBclemens.tolboom
core-config_comparer-storage_comparer.patch924 bytesclemens.tolboom

Comments

Anonymous’s picture

Anonymous (not verified) commented
Issue tags: +Configuration system

this seems legit to me.

did you look for any other occurrences of 'storage_comparer'?

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented
Title: Naming confusion between config_comparer and storage_comparer in config.admin.inc » Naming confusion between config_comparer and storage_comparer in config.admin.inc and other places
StatusFileSize
new core-config_comparer-storage_comparer-2012566-2.patch3.29 KB

@beejeebus: good point. Scanning code there where a few more occurrences.

I now have renamed differently to storage_comparer as that resembles more this it's class.

Status: Needs review » Needs work
Issue tags: -Configuration system

The last submitted patch, core-config_comparer-storage_comparer-2012566-2.patch, failed testing.

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
commented
Status: Needs work » Needs review
Issue tags: +Configuration system

#2: core-config_comparer-storage_comparer-2012566-2.patch queued for re-testing.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented
Status: Needs review » Needs work
Issue tags: +Novice, +Needs reroll

Tagging

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
commented
Issue summary: View changes

Changed the preferred name from config_comparer to storage_comparer.

Luka Zitnik’s picture

Luka Zitnik commented
Assigned: Unassigned » Luka Zitnik
Issue summary: View changes

Taking on this issue.

commit 0adc91af5c22fedfa74ad1c66cea42a3b6eb5c4e removed /core/modules/config/config.admin.inc, but there are still few $config_comparer variables left for me to rename them to $storage_comparer.

Luka Zitnik’s picture

Luka Zitnik commented
Status: Needs work » Needs review
StatusFileSize
new core-config_comparer-storage_comparer-2012566-7.patch3.4 KB

Renamed local variables $config_comparer, instances of \Drupal\Core\Config\StorageComparer, to $storage_comparer.

Status: Needs review » Needs work

The last submitted patch, 7: core-config_comparer-storage_comparer-2012566-7.patch, failed testing.

Luka Zitnik’s picture

Luka Zitnik commented
Status: Needs work » Needs review

7: core-config_comparer-storage_comparer-2012566-7.patch queued for re-testing.

Luka Zitnik’s picture

Luka Zitnik commented
Assigned: Luka Zitnik » Unassigned
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Status: Needs review » Reviewed & tested by the community

Good change to make the variable name consistent.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 77d0c14 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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