Problem/Motivation

#778346: system_sort_modules_by_info_name() is misnamed added a new functional method but that's not the Drupal 8 & 9 way - in this instance we could add the array callback method as a static to the ExtensionList class.

Also #778346: system_sort_modules_by_info_name() is misnamed didn't add a deprecation message to the docblock and the the docblock missed the point of the name change - which was to make it obvious that this method is generic.

Proposed resolution

Move the method to the ExtensionList class and use it from there. Remove system_sort_by_info_name() if we manage to fix this before 9.3.x.

Remaining tasks

User interface changes

API changes

system_sort_by_info_name() (9.3.x only) is removed.
system_sort_modules_by_info_name() is deprecated in favour of \Drupal\Core\Extension\ExtensionList::sortByName()

Update https://www.drupal.org/node/3225624 on commit.

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#10 interdiff_7-10.txt1.95 KBsrilakshmier
#10 3225779-10.patch8.48 KBsrilakshmier
#7 interdiff_5-7.txt1.22 KBravi.shankar
#7 3225779-7.patch8.48 KBravi.shankar
#5 3225779-5.patch8.47 KBalexpott
#5 2-5-interdiff.txt2.57 KBalexpott
#2 3225779-2.patch7.26 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3225779-2.patch7.26 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Related issues: +#778346: system_sort_modules_by_info_name() is misnamed

Status: Needs review » Needs work

The last submitted patch, 2: 3225779-2.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Needs review
FileSize
2-5-interdiff.txt2.57 KB
3225779-5.patch8.47 KB
daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work 
  1. +++ b/core/lib/Drupal/Core/Extension/ExtensionList.php
@@ -580,4 +580,20 @@ public function checkIncompatibility($name) {
+  public static function sortByName(Extension $a, Extension $b) {

    Can we add a return typehint to the method. 

  2. +++ b/core/modules/system/system.module
@@ -891,16 +891,14 @@ function system_region_list($theme, $show = REGIONS_ALL) {
+ * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use

    It should be 9.3.0 not 9.1.0.

ravi.shankar’s picture

ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
3225779-7.patch8.48 KB
interdiff_5-7.txt1.22 KB

Made changes as suggest in comment #6, please review.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work
Issue tags: -Needs change record updates +Novice 
+++ b/core/modules/system/system.module
@@ -891,16 +891,14 @@ function system_region_list($theme, $show = REGIONS_ALL) {
+  @trigger_error('system_sort_modules_by_info_name() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Use \Drupal\Core\Extension\ExtensionList::sortByName() instead. See https://www.drupal.org/node/3225624', E_USER_DEPRECATED);

+++ b/core/modules/system/tests/src/Kernel/SystemDeprecationTest.php
@@ -19,13 +19,15 @@ class SystemDeprecationTest extends KernelTestBase {
+    $this->expectDeprecation('system_sort_modules_by_info_name() is deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Use \Drupal\Core\Extension\ExtensionList::sortByName() instead. See https://www.drupal.org/node/3225624');

I have created a new CR for this issue. Please change the link to the new CR (https://www.drupal.org/node/3225999).

srilakshmier’s picture

srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commented
Assigned: Unassigned » srilakshmier
srilakshmier’s picture

srilakshmier CreditAttribution: srilakshmier at Valuebound for Valuebound commented
Assigned: srilakshmier » Unassigned
Status: Needs work » Needs review
FileSize
3225779-10.patch8.48 KB
interdiff_7-10.txt1.95 KB

Uploaded the patch based on comment #8. Please review.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice

The old function is deprecated and has a deprecation message test.
This issue does the same as #778346: system_sort_modules_by_info_name() is misnamed, only the replacement method is different.
Therefor if this get committed, then the CR (https://www.drupal.org/node/3225624) needs to be unpublished.
For me it is RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 22a6932 and pushed to 9.3.x. Thanks!

  • catch committed 22a6932 on 9.3.x 
    Issue #3225779 by alexpott, srilakshmier, ravi.shankar, daffie: Provide...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Unpublished https://www.drupal.org/node/3225624

Status: Fixed » Closed (fixed)

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