Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #2504025 by Fabianx, Pol: Add static container factory method for plugins (D8 style) #72

Merged
merged 15 commits into from
Jun 24, 2015
Merged

Issue #2504025 by Fabianx, Pol: Add static container factory method for plugins (D8 style) #72

merged 15 commits into from
Jun 24, 2015

Conversation

drupol
Copy link
Collaborator

@drupol drupol commented Jun 23, 2015

No description provided.

$plugin_class = static::getPluginClass($plugin_id, $plugin_definition);

// If the plugin provides a factory method, pass the container to it.
if (is_subclass_of($plugin_class, 'Drupal\service_container\Plugin\ContainerAwarePluginManager')) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be the Interface that I outlined, not our own.

We are okay to add that interface, too.

@drupol
Copy link
Collaborator Author

drupol commented Jun 24, 2015

Investigating...

There were 2 errors:

  1. Drupal\Tests\service_container\Plugin\ContainerAwarePluginManagerTest::test_createInstance
    Drupal\Component\Plugin\Exception\PluginException: Plugin (block) instance class "Mockery_11__Drupal_render_cache_block_RenderCache_Controller_BlockController#0000000034594da8000000002ba5df98" does not exist.
    /home/travis/build/LionsAd/service_container/src/Plugin/ContainerAwarePluginManager.php:120
    /home/travis/build/LionsAd/service_container/src/Plugin/ContainerAwarePluginManager.php:63
    /home/travis/build/LionsAd/service_container/tests/src/Plugin/ContainerAwarePluginManagerTest.php:58
  2. Drupal\Tests\service_container\Plugin\ContainerAwarePluginManagerTest::test_getInstance
    Drupal\Component\Plugin\Exception\PluginException: Plugin (block) instance class "Mockery_11__Drupal_render_cache_block_RenderCache_Controller_BlockController#0000000034594dca000000002ba5df98" does not exist.
    /home/travis/build/LionsAd/service_container/src/Plugin/ContainerAwarePluginManager.php:120
    /home/travis/build/LionsAd/service_container/src/Plugin/ContainerAwarePluginManager.php:63
    /home/travis/build/LionsAd/service_container/src/Plugin/ContainerAwarePluginManager.php:89
    /home/travis/build/LionsAd/service_container/tests/src/Plugin/ContainerAwarePluginManagerTest.php:68

* @inheritdoc
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
return 'Hello World!';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that is wrong.

This needs to be:

return new static($configuration, $plugin_id, $plugin_definition, 'Hello World');

then add a __construct here, that populates $this->data and add a getData() method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I wrote that because I didn't know how to test it correctly.

@LionsAd
Copy link
Owner

LionsAd commented Jun 24, 2015

RTBM

@LionsAd LionsAd added the RTBM label Jun 24, 2015
@LionsAd
Copy link
Owner

LionsAd commented Jun 24, 2015

Just waiting for tests and coverage report.

@LionsAd LionsAd changed the title Update createInstance() method. Add static container factory method for plugins (D8 style) Jun 24, 2015
@LionsAd LionsAd changed the title Add static container factory method for plugins (D8 style) Issue #2504025 by Fabianx, Pol: Add static container factory method for plugins (D8 style) Jun 24, 2015
LionsAd added a commit that referenced this pull request Jun 24, 2015
…ntainerAwarePluginManager

Issue #2504025 by Fabianx, Pol: Add static container factory method for plugins (D8 style)
@LionsAd LionsAd merged commit 98359a3 into LionsAd:7.x-1.x Jun 24, 2015
@drupol drupol deleted the Update-createInstance-method-of-ContainerAwarePluginManager branch June 25, 2015 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants