As said in #1780396-64: Namespaces of disabled modules are registered during test runs, removing Drupal\edit\Plugin\EditorManager::processDefinition() was forgotten in #1836008: Remove drupal_classloader() use in Drupal\Core\AnnotatedClassDiscovery.

This is a tiny patch that follows the pattern established by #1836008: Remove drupal_classloader() use in Drupal\Core\AnnotatedClassDiscovery. Originally this was going to be removed by #1903346: Establish a new DefaultPluginManager to encapsulate best practices, but that turns out to no longer be true. Hence this small new issue.

CommentFileSizeAuthor
#8 cleanup_module_annotations-2006278-8.patch11.66 KBWim Leers
#1 cleanup_module_annotations-2006278-1.patch11.42 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Title: Remove leftover plugin manager processDefinition() to deal with disabled modules while running tests » Remove leftover 'module' annotation keys and plugin manager processDefinition() to deal with disabled modules during tests
Status: Active » Needs review
FileSize
cleanup_module_annotations-2006278-1.patch11.42 KB

So it turns out that the 'module' keys were not removed from the "CKEditor" and "Editor" modules either, for an unknown reason. They should have been removed in #1836008: Remove drupal_classloader() use in Drupal\Core\AnnotatedClassDiscovery where possible. E.g. the block and entity plugins use them, but Edit nor Editor nor CKEditor does.

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc commented

if that comes back green, it looks super sane to me. RTBC pending green.

Eclipse

Status: Needs review » Needs work
Issue tags: -sprint, -Spark

The last submitted patch, cleanup_module_annotations-2006278-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
Issue tags: +sprint, +Spark

#1: cleanup_module_annotations-2006278-1.patch queued for re-testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Reviewed & tested by the community

Looks good.

Dries’s picture

Dries CreditAttribution: Dries commented

#1: cleanup_module_annotations-2006278-1.patch queued for re-testing.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Needs work

Patch no longer applies so asking for a re-test.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Status: Needs work » Needs review
FileSize
cleanup_module_annotations-2006278-8.patch11.66 KB
effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

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

Committed ba80bc3 and pushed to 8.x. Thanks!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: -sprint

.

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