CommentFileSizeAuthor
#16 interdiff_9-16.txt2.8 KBhash6
#16 3113053-16.patch5.74 KBhash6
#12 interdiff_9-12.txt3.4 KBhash6
#12 3113053-12.patch6.03 KBhash6
#9 3113053-9.patch2.77 KBlongwave
#2 3113053-2.patch5.96 KBHardik_Patel_12
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Hardik_Patel_12 created an issue. See original summary.

Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented
FileSize
3113053-2.patch5.96 KB

Kindly review a patch.

Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented
Status: Needs work » Needs review
Parent issue: » #2716163: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 9 branch
Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented
Assigned: Hardik_Patel_12 » Unassigned
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented
Assigned: Unassigned » naresh_bavaskar
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented
Assigned: naresh_bavaskar » Unassigned
Status: Needs review » Reviewed & tested by the community

Above patch applied successfully, LGTM

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Reviewed & tested by the community » Needs work

Needs work due to all tests failing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Title: Remove all @deprecated code from Theme component » Remove $baseThemes BC layer from \Drupal\Core\Theme\ActiveTheme
Related issues: +#3069052: Properly deprecate the stylesheets-remove key from theme info.yml files.

@naresh_bavaskar - the tests in #2 have completely failed. "Build Successful" is not the same as tests passed :)

I would leave out the stylesheets remove stuff - that needs more work to do properly - and much of this will be done in #3069052: Properly deprecate the stylesheets-remove key from theme info.yml files.

This issue can deal with the removal of \Drupal\Core\Theme\ActiveTheme::$baseThemes

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
3113053-9.patch2.77 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs review » Needs work

@longwave you need to remove core/tests/Drupal/KernelTests/Core/Theme/ActiveThemeLegacyTest.php too. It's a good idea to search the codebase for deprecation messages when removing an @trigger_error

hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
Assigned: Unassigned » hash6
hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
FileSize
3113053-12.patch6.03 KB
interdiff_9-12.txt3.4 KB

Updated the Patch
- Removed the ActiveThemeLegacyTest.php
- Removed the getStyleSheetsRemove() in ActiveTheme.php

hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
Status: Needs work » Needs review
hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
Assigned: hash6 » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

@hash6 the style sheets remove stuff should be left alone for now there is a separate issue for that - see earlier comments and issue title

hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
Assigned: Unassigned » hash6
FileSize
3113053-16.patch5.74 KB
interdiff_9-16.txt2.8 KB
hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
Assigned: hash6 » Unassigned
hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
hash6’s picture

hash6 CreditAttribution: hash6 at QED42 commented
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Thanks for removing the obsolete test.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented 
+++ b/core/lib/Drupal/Core/Theme/ActiveTheme.php
@@ -138,13 +127,6 @@ public function __construct(array $values) {
-    if (!empty($values['base_themes']) && empty($this->baseThemeExtensions)) {
-      @trigger_error("The 'base_themes' key is deprecated in Drupal 8.7.0  and support for it will be removed in Drupal 9.0.0. Use 'base_theme_extensions' instead. See https://www.drupal.org/node/3019948", E_USER_DEPRECATED);

There was not much time passed since deprecation so better to replace it with assert()

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

@andypost I don't think so - actually constructing your own ActiveTheme object is not really done in contrib.

Committed d57f7eb and pushed to 9.0.x. Thanks!

  • alexpott committed d57f7eb on 9.0.x 
    Issue #3113053 by hash6, Hardik_Patel_12, longwave, alexpott: Remove $...

Status: Fixed » Closed (fixed)

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