Contrib.social

Fix incorrect use of word 'component' in locale module

Open on Drupal.org →
Created on 21 February 2024, over 1 year ago
Updated 28 February 2024, over 1 year ago

Problem/Motivation

Locale module uses the word 'component' in both docs and code to mean different things, and most if not all of them are wrong.

Here's a sample:

  /**
   * Get all configuration names and folders for a list of modules or themes.
   *
   * @param string $type
   *   Type of components: 'module' | 'theme' | 'profile'
   * @param array $list
   *   Array of theme or module names.
   *
   * @return array
   *   Configuration names provided by that component. In case of language
   *   module this list is extended with configured languages that have
   *   predefined names as well.
   */
  public function getComponentNames($type, array $list) {

The docblock first line says:

> Get all configuration names

The method returns names of configuration objects. But it's called 'getComponentNames' -- are components configuration objects? I don't think anywhere else calls them that.

The parameter docs says:

> * Type of components: 'module' | 'theme' | 'profile'

and the @return says:

> Configuration names provided by that component

So components are module/theme/profiles?

We're now using the word 'component' to mean two different things!

There are other places in the module that do this too, such as this comment:

      // Update active configuration copies of all prior shipped configuration if
      // they are still English. It is not enough to change configuration shipped
      // with the components just installed, because installing a component such
      // as views may bring in default configuration from prior components.

Steps to reproduce

Proposed resolution

Replace the word 'component' with either:

- 'configuration object' -- this is what the core config component calls them
- 'extension'

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Locale  →

Last updated about 21 hours ago

Created by

🇬🇧United Kingdom joachim

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joachim
  • First commit to issue fork.
  • Merge request !6779Issue #3422955: Fix incorrect use of word 'component' in locale module → (Open) created by govind_giri_goswami
  • Pipeline finished with Success
    over 1 year ago
    Total: 624s
    #104625
  • Pipeline finished with Success
    over 1 year ago
    Total: 645s
    #104626
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India govind_giri_goswami

    update comments in core/modules/locale/src/LocaleConfigManager.php file and core/modules/locale/src/LocaleDefaultConfigStorage.php file

  • Comment over 1 year ago →
    🇮🇳India mithun s Bangalore

    Changes looks good. Since we are updating only the text on comments, I don't anticipate regression.
    Thank you!

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India karanpagare

    I can see more references also in locale.bulk.inc and locale.module which can be changed.

  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India adwivedi008

    Worked on #6 as suggested by @karanpagare
    Moving the issue to needs review

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇬🇧United Kingdom joachim

    Looks good so far, but method names such as getComponentNames need changing too.

  • Pipeline finished with Success
    over 1 year ago
    Total: 475s
    #104777
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India karanpagare

    Updated getComponentNames as per #9, can review.

  • Pipeline finished with Failed
    over 1 year ago
    Total: 633s
    #105574
  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This caused test failures, core/modules/locale/src/LocaleConfigManager.php isn't internal so I think changing function names could have BC implications.

  • Pipeline finished with Failed
    over 1 year ago
    Total: 619s
    #106595
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024