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

Update plugin system by using the PluginBase from D8 #5

Merged
merged 5 commits into from Jun 26, 2015
Merged

Update plugin system by using the PluginBase from D8 #5

merged 5 commits into from Jun 26, 2015

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Jun 16, 2015

Remove the tests related to the old BasePlugin.
Replaced the method getType() by getPluginId().

…version of service_container.

Remove the test related to the old BasePlugin.
Replaced the method getType() by getPluginId().
@drupol
Copy link
Contributor Author

drupol commented Jun 16, 2015

PHP Fatal error: Class 'Drupal\Component\Plugin\PluginBase' not found in /home/travis/build/LionsAd/render_cache/src/RenderCache/Controller/AbstractBaseController.php on line 16

But the module service_container is a dependency.

@LionsAd
Copy link
Owner

LionsAd commented Jun 16, 2015

Yeah, we first need to update service_container-7.x-1.x-dev on d.org, before this will work.

@LionsAd
Copy link
Owner

LionsAd commented Jun 16, 2015

Will do so today, the PR looks great!

@drupol
Copy link
Contributor Author

drupol commented Jun 16, 2015

But I don't understand why the tests are failing. It should work if service_container (dev or not, because PluginBase is in there) is a dependency. What is wrong ?

@LionsAd
Copy link
Owner

LionsAd commented Jun 16, 2015

service_container still calls the plugins with different arguments.

@LionsAd
Copy link
Owner

LionsAd commented Jun 16, 2015

Oh and for unit tests we need the same fix of having tests/composer.json depend on service_container: 'dev-7.x-1.x'.

@LionsAd
Copy link
Owner

LionsAd commented Jun 16, 2015

See tests/composer.json in OpenLayers.

@drupol
Copy link
Contributor Author

drupol commented Jun 16, 2015

Well done 👍 seems that tests are ok now... let's wait the end.

"Drupal\\Component\\": "lib/"
"Drupal\\Component\\": "lib/",
"Drupal": "../lib/",
"Psr": "../lib/"
Copy link
Owner

Choose a reason for hiding this comment

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

Can we revert those changes, please?

Copy link
Owner

Choose a reason for hiding this comment

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

I mean just those 3 lines

@drupol
Copy link
Contributor Author

drupol commented Jun 26, 2015

Just updated code to have it working with latest version of Service Container, hopefully the syntax won't change from now.

@drupol
Copy link
Contributor Author

drupol commented Jun 26, 2015

1 timeout, 7 greens on 8 tests.

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

LionsAd commented Jun 26, 2015

Thanks, @drupol

LionsAd added a commit that referenced this pull request Jun 26, 2015
Update plugin system by using the PluginBase from D8
@LionsAd LionsAd merged commit eae760d into LionsAd:7.x-2.x Jun 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants