Problem/Motivation

Although Language is not an entity, it almost shares a structure with ConfigurableLanguage and that uses label (just as other entities do).

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because there is no error due to this. Is for consistant DX.
Issue priority Normal because is only effecting Language. (Not major; is not "important by community consensus")
Unfrozen changes Not unfrozen because is not only changing css, tests, markup, etc.
Prioritized changes Might qualify for prioritized change under "reduces fragility", but probably not.
Disruption Not disruptive for core/contributed and custom modules/themes if it retains public method names for BC, but marks them deprecated.

Postponed for 8.1.x because it does not meet the qualification on https://www.drupal.org/core/beta-changes ?

Proposed resolution

  • In core/lib/Drupal/Core/Language/Language.php change name to label to be consistent.
  • Update all uses.
  • Keep old function names as a BC wrapper.
  • Need a follow up issue postponed until 8.1.x. to remove the deprecated methods.

Remaining tasks

User interface changes

No

API changes

No

Related issues

CommentFileSizeAuthor
#32 2246721-language_label-32.patch8.54 KByogeshmpawar
#30 2246721-language_label-30.patch8.54 KByogeshmpawar
#27 2246721-language_label-27.patch8.53 KBmartin107
#27 interdiff-25.27.txt464 bytesmartin107
#25 doOver-2246721-25.patch8.55 KBmartin107
#9 interdiff-7-9.txt4.04 KBmartin107
#9 2246721-language_label-9.patch6.91 KBmartin107
#7 2246721-language_label-7.patch3.55 KBmartin107
#2 2246721-language_label-2.patch3.64 KBYesCT
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

FWIW, I personally think label should be renamed to name instead, but I don't really care either.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Status: Active » Needs review
FileSize
2246721-language_label-2.patch3.64 KB

here is a start. doing just renaming of name to label.

one thing I dont get is why this didn't have to have label(). (and id())

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Related issues: +#2246657: $options is misnamed in Language::__construct() should be property values

will conflict with #2246657: $options is misnamed in Language::__construct() should be property values, but that is fine. can just re-roll one when the other gets in.

Status: Needs review » Needs work

The last submitted patch, 2: 2246721-language_label-2.patch, failed testing.

YesCT’s picture

YesCT CreditAttribution: YesCT commented

might be a duplicate of #2246679: Make Language module's LanguageInterface (to be ConfigurableLanguageInterface) extend Core's LanguageInterface

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Related issues: +#2246695: replace all core usages of id() with getid()
martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Needs review
FileSize
2246721-language_label-7.patch3.55 KB

This label/name thing is causing confusion in other issues. ... so I think it is worth fixing here first.

For now just a straight reroll.

Status: Needs review » Needs work

The last submitted patch, 7: 2246721-language_label-7.patch, failed testing.

martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Needs review
FileSize
2246721-language_label-9.patch6.91 KB
interdiff-7-9.txt4.04 KB

Fixed fatal error preventing unit tests.

Plus fixed a smattering of the errors.. at least now more tests will run.

Still a long way to go.

Status: Needs review » Needs work

The last submitted patch, 9: 2246721-language_label-9.patch, failed testing.

martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Needs review

#9 patch was testing in the middle of a commit spree .. CommentBlockTest.php is locally passing.. retesting just in case.

martin107 queued 9: 2246721-language_label-9.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: 2246721-language_label-9.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Entity Field API

The opposite approach is suggested in #2246695: replace all core usages of id() with getid()

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Related issues: +#2199917: Implement EntityLabeledInterface
YesCT’s picture

YesCT CreditAttribution: YesCT commented

note #2341341: Change public 'name' property access on languages to getName() and add back setName() went in.

Also, might need some kind of plan like the 3 issue set of #2350807: add getId() and make id() a wrapper for it and deprecate it
but I'm not sure. it's been awhile since I looked at this.

martin107’s picture

martin107 CreditAttribution: martin107 commented

@YestCT, you are more aware of the wider context .... should we resolve this in 8.0.x or 8.1.x?

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes
Issue tags: +Needs issue summary update, +Needs reroll

needs beta evaluation. adding instructions to summary and tag to update issue summary

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Title: have Language use property label to be consistent with other entities » Language class should use property 'label' to be consistent with entities
Issue summary: View changes
Issue tags: -Needs issue summary update +SprintWeekend2015Queue

Updated issue summary and title.

YesCT’s picture

YesCT CreditAttribution: YesCT commented 
  1. +++ b/core/lib/Drupal/Core/Language/Language.php
@@ -33,11 +33,11 @@ class Language implements LanguageInterface {
-  public $name = '';
+  public $label = '';

    name is protected now. so internal refactoring like that would not cause a backwards compatibility (BC) break. 

  2. +++ b/core/lib/Drupal/Core/Language/Language.php
@@ -119,15 +119,15 @@ public function __construct(array $values = array()) {
-  public function getName() {
-    return $this->name;
+  public function getLabel() {
+    return $this->label;

    but this would be a BC break.

    We could probably add getLabel, and make getName a wrapper for that, and mark it deprecated for removal in 8.1.x

---
but I dont think removing getName would be allowed at this point in the beta.

---

I'll do a beta evaluation and update the summary.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes
YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue tags: -SprintWeekend2015Queue
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Version: 8.0.x-dev » 9.x-dev

If we don't believe this is allowed in beta, then it will be even less allowed in RC. In point releases, API changes are also avoided as possible. Then this will need to be Drupal 9 to avoid the BC break later.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.x-dev » 8.3.x-dev

Adding a new method and deprecating the old one is fine in a minor.

Also protected properties are explicitly @internal so changing that is fine.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
doOver-2246721-25.patch8.55 KB

@catch thanks for the @internal comment ... that is good to know.

The last patch is old ... in the mean time the properties have become protected so the @internal argument seems irrelevant.

When rerolling a 14 line patch and there are 14 conflicts ... well I have just started over.

ConfigurableLanguageUnitTest and LanguageUnitTest are green locally, so fingers crossed, but I suspect there may be at least one little gremlin.

Status: Needs review » Needs work

The last submitted patch, 25: doOver-2246721-25.patch, failed testing.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-25.27.txt464 bytes
2246721-language_label-27.patch8.53 KB

fixed.

Status: Needs review » Needs work

The last submitted patch, 27: 2246721-language_label-27.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented 
  1. +++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -190,14 +190,30 @@ protected static function getDefaultLangcode() {
+  public function getLabel() {
+    return $this->label;

    btw Annotation should be changed also 

  2. +++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -190,14 +190,30 @@ protected static function getDefaultLangcode() {
   public function setName($name) {
-    $this->label = $name;
+    $this->setLabel($name);
+
+    return $this;

    should be oneliner
    return $this->setLabel()

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location 🇮🇳 Pune(MH)
CreditAttribution: yogeshmpawar as a volunteer and at QED42 commented
Status: Needs work » Needs review
FileSize
2246721-language_label-30.patch8.54 KB

I have rerolled the patch as per comment #30

Status: Needs review » Needs work

The last submitted patch, 30: 2246721-language_label-30.patch, failed testing.

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location 🇮🇳 Pune(MH)
CreditAttribution: yogeshmpawar as a volunteer and at QED42 commented
Status: Needs work » Needs review
FileSize
2246721-language_label-32.patch8.54 KB

Fixed the Cl error.

Status: Needs review » Needs work

The last submitted patch, 32: 2246721-language_label-32.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.