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

Added Filter Provider #26

Closed
wants to merge 1 commit into from
Closed

Added Filter Provider #26

wants to merge 1 commit into from

Conversation

igorgolovanov
Copy link

No description provided.

@@ -35,7 +35,7 @@ public function init($event)
$serviceListener->addServiceManager(
'FilterManager',
'filters',
'Zend\ModuleManager\Feature\FilterProviderInterface',
FilterProviderInterface::class,
Copy link
Member

Choose a reason for hiding this comment

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

Those are different namespaces

Copy link
Member

Choose a reason for hiding this comment

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

@Maks3w This is being done to remove the various provider classes from zend-modulemanager; this PR (and others that @golovanov has provided today) are simply porting those providers to the relevant components, and modifying the ServiceListener configuration to reference the new provider.

The primary problem with the approach, however, is that if users were implementing the old interface, they now need to switch to the new one. That said, the ServiceListener looks for either the correct interface implementation, or the method it defines, so functionality should continue to work. The issue will be if we then remove the related interface from zend-modulemanager, as that situation would break existing code.

Copy link
Member

Choose a reason for hiding this comment

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

There is another option for relax dependencies without break the code.

We could store in this repo the interface Zend\ModuleManager\Feature\FilterProviderInterface and setup the file in composer.json as a classmap or as another source of Zend\ModuleManager namespace. Probably we should add lower versions of zend-modulemanager as conflict for to avoid double declaration of the same file.

weierophinney added a commit that referenced this pull request Apr 11, 2018
weierophinney added a commit that referenced this pull request Apr 11, 2018
@weierophinney
Copy link
Member

Thanks, @igorgolovanov! Merged to develop, to be released with 2.8.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants