Problem/Motivation

DefaultConfigTest has some leftover @todos pointing to #2208429: Extension System, Part III: ExtensionList, ModuleExtensionList and ProfileExtensionList and #2186491: [meta] D8 Extension System: Discovery/Listing/Info.

The first issue is now fixed, and it seems that we can remove both workarounds.

Proposed resolution

Remove the dead code.

Remaining tasks

Review the patch.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
FileSize
1023 bytes

Spotted this while working on some other unrelated things :)

andypost’s picture

Got spot but there's more

core8$ git grep 2208429
core/lib/Drupal/Core/Extension/ModuleInstaller.php:299:        // @see https://www.drupal.org/node/2208429
core/lib/Drupal/Core/Extension/ModuleInstaller.php:471:      // @see https://www.drupal.org/node/2208429
core/lib/Drupal/Core/Extension/ThemeHandler.php:333:    // @see https://www.drupal.org/node/2208429
core/modules/system/tests/src/Kernel/Extension/ModuleHandlerTest.php:29:    //   system.module, see https://www.drupal.org/node/2208429.
core/tests/Drupal/KernelTests/Config/DefaultConfigTest.php:54:    //   system.module, see https://www.drupal.org/node/2208429.
BramDriesen’s picture

Status: Needs review » Needs work

Setting to needs work for #3

amateescu’s picture

Title: Remove a couple of workarounds from DefaultConfigTest » Remove a couple of workarounds from DefaultConfigTest and ModuleHandlerTest
Status: Needs work » Needs review
FileSize
2.03 KB
1.04 KB

@andypost, good point :) Removed the same workarounds from ModuleHandlerTest.

The ThemeHandler part is being done in #2659940: Extension System, Part III: ThemeExtensionList and ThemeEngineExtensionList, as for ModuleInstaller I couldn't find the exact issue where that's supposed to be cleaned up, but it's certainly not in the scope of this one.

BramDriesen’s picture

Status: Needs review » Reviewed & tested by the community

I think this captures everything of the scope of this issue :) code removal looks good.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed a11f188 and pushed to 8.7.x. Thanks!

  • catch committed a11f188 on 8.7.x
    Issue #3014011 by amateescu, andypost: Remove a couple of workarounds...

Status: Fixed » Closed (fixed)

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