Fix incorrect use of word 'component' in locale module

Created on 21 February 2024, 4 months ago
Updated 28 February 2024, 4 months 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 2 days 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.
  • Pipeline finished with Success
    4 months ago
    Total: 624s
    #104625
  • Pipeline finished with Success
    4 months ago
    Total: 645s
    #104626
  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia govind_giri_goswami

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

  • ๐Ÿ‡ฎ๐Ÿ‡ณ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 4 months 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 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia adwivedi008

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

  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

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

  • Pipeline finished with Success
    4 months ago
    Total: 475s
    #104777
  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia karanpagare

    Updated getComponentNames as per #9, can review.

  • Pipeline finished with Failed
    4 months ago
    Total: 633s
    #105574
  • Status changed to Needs work 4 months 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
    4 months ago
    Total: 619s
    #106595
Production build 0.69.0 2024