-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix delegators to allow usage in plugin managers #5175
fix delegators to allow usage in plugin managers #5175
Conversation
* @return mixed | ||
* @throws Exception\ServiceNotCreatedException | ||
*/ | ||
protected function createDelegatorFromFactory($canonicalName, $requestedName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is indeed clean, consider that I inlined the code for performance reasons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm but it's 40 lines of code now, plus there are separated methods for each service type. this way it's not only cleaner, it's more consistent too.
are you sure the performance gain is worth it? maybe there is even another way to improve that aspect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm... it's probably ok to keep the method separate then, yep.
on Should be trivial though:
|
yep, why not, this way we can keep delegator usage in line with the other service types :) |
done: * move delegator logic from create() to separate method * remove automatic invokable service for delegator factories * add delegator factory class validation * update tests
fix delegators to allow usage in plugin managers Conflicts: tests/ZendTest/ServiceManager/ServiceManagerTest.php
…si/fix/pm-delegators fix delegators to allow usage in plugin managers Conflicts: tests/ZendTest/ServiceManager/ServiceManagerTest.php
see #5172