Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett’s picture

Issue tags: +Plugin system
FileSize
3.97 KB

Here's just the changes for this issue.

tim.plunkett’s picture

Well, why bother with a one-off discovery that is not reusable?

tim.plunkett’s picture

FileSize
2.88 KB

That other patch went in.

tim.plunkett’s picture

Issue tags: +Annotation

Tagging.

dawehner’s picture

+++ b/core/modules/tour/lib/Drupal/tour/TipPluginManager.phpundefined
@@ -25,7 +25,8 @@ class TipPluginManager extends PluginManagerBase {
+    $annotation_namespaces = array('Drupal\tour\Annotation' => DRUPAL_ROOT . '/core/modules/tour/lib');

A general point I want to raise here: This removes the possiblity to override the tour module, by putting it into /modules instead of /core/modules. I'm not sure whether this is still possible at the moment in D8, but it was certainly in previous time.

EclipseGc’s picture

Overriding the Manager class should still be possible, and if that's possible, then this line can be changed. No?

Eclipse

tim.plunkett’s picture

Issue tags: -Plugin system, -Annotation

#4: tip-1967406-4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, tip-1967406-4.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: +Plugin system, +Annotation

#4: tip-1967406-4.patch queued for re-testing.

dawehner’s picture

Overriding the Manager class should still be possible, and if that's possible, then this line can be changed. No?

Sure but do you expect that you can't move drupal modules but you require to alter the manager? What about using basename or __DIR__ or something like that here?

tim.plunkett’s picture

FileSize
898 bytes
2.89 KB

Borrowing a trick from @EclispeGC, this does exactly the same thing.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

That's a great tip!!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed a78b614 and pushed to 8.x. Thanks!

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