Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Conversation

@pine3ree
Copy link
Contributor

No description provided.

@samsonasik
Copy link
Contributor

I think the delegator factory class need to be updated as well to implements DelegatorFactoryInterface

@pine3ree
Copy link
Contributor Author

Hello @samsonasik, I agree...

Copy link
Contributor

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

use DelegatorFactoryInterface from ServiceManager\Factory namespace

use Zend\Stratigility\MiddlewarePipe;

class ApiResourcePipelineDelegatorFactory
class ApiResourcePipelineDelegatorFactory implements DelegatorFactoryInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

it is better to use DelegatorFactoryInterface from ServiceManager\Factory namespace so we use :

public function __invoke(
      ContainerInterface $container,
      $name,
      callable $callback,
      array $options = null
)

@pine3ree
Copy link
Contributor Author

pine3ree commented Sep 17, 2016

@samsonasik
Yes, right. That was still the deprecated delegator factory. Thanks.

added  blank lines for clarity in the example (as it was done in the ApiResourcePipelineDelegatorFactory example):
- retrieve the service
- alter/extend the service
- return the altered/extended service
(should we add inline code comments?)
@pine3ree
Copy link
Contributor Author

pine3ree commented Sep 17, 2016

@samsonasik
Done. I hope I didn't miss anything this time. Please verify, :-) thank You. Kind regards.

@samsonasik
Copy link
Contributor

it seems ok now 👍

@weierophinney
Copy link
Member

There's a problem with this patch: the original documentation was demonstrating the zend-servicemanager v2 API, but the changes introduced show only the v3 API. The Expressive installer, and the default configuration, supports both. Ideally, we should be:

  • implementing Zend\ServiceManager\DelegatorFactoryInterface, as originally outlined, but
  • implementing the methods in both the v2 and v3 versions of the interface.

This means more code in the examples, but will ensure that the code, if copy and pasted into an application, will work regardless of the zend-servicemanager version installed by the user.

If I can get back to this today, I'll use your changes as a starting point. If you do not see activity from me, and you have a chance to update, please do.

Thanks!

@pine3ree
Copy link
Contributor Author

@weierophinney , sure! kind regards!

@weierophinney weierophinney self-assigned this Nov 11, 2016
weierophinney added a commit that referenced this pull request Nov 11, 2016
@weierophinney
Copy link
Member

@pine3ree I'd updated the zend-form examples with a previous documentation patch, and have now pushed another patch that updates the route-specific pipeline examples. Both now demonstrate both v2 and v3 compatibility for the delegator factory examples.

@pine3ree pine3ree deleted the patch-8 branch November 11, 2016 14:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants