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

Notice: Undefined variable: serviceLocator #149

Closed
wants to merge 3 commits into from
Closed

Notice: Undefined variable: serviceLocator #149

wants to merge 3 commits into from

Conversation

michalbundyra
Copy link
Member

@weierophinney here it is: fix for #148. I've added some tests, not sure if they are useful because as I can see this part is removed from v3. Let me know please what do you think. Thanks.

@weierophinney weierophinney added this to the 2.7.9 milestone Jun 9, 2016
@weierophinney weierophinney self-assigned this Jun 9, 2016
@weierophinney
Copy link
Member

Okay, I see a couple of things.

First, as you note, the functionality is removed in zend-servicemanager v3, which means the tests fail if that version is installed.

Interestingly, zend-servicemanager-di, which is where we moved the functionality for the zend-mvc v3 release, is tested against both versions, and contains the exact same functionality, albeit with different namespaces. Unfortunately, it contains the same typo. 😦 I'll be doing a PR for that momentarily so we can get a working version in place.

My thought is that we do the following:

  • Modify DiAbstractServiceFactoryFactory to extend Zend\ServiceManager\Di\DiAbstractServiceFactoryFactory
  • Do the same for DiAbstractServiceFactory (have it extend the zend-servicemanager-di variant)
  • Add a requirement in composer.json for zend-servicemanager-di

The above will solve the issue so that zend-mvc v2 continues to work, both against zend-servicemanager v2 and v3.

weierophinney added a commit to zendframework/zend-servicemanager-di that referenced this pull request Jun 9, 2016
weierophinney added a commit to zendframework/zend-servicemanager-di that referenced this pull request Jun 9, 2016
weierophinney added a commit to zendframework/zend-servicemanager-di that referenced this pull request Jun 9, 2016
…-servicemanager-di

(class marked as deprecated in zend-mvc, removed with 3.0 release)
@michalbundyra
Copy link
Member Author

@weierophinney I've updated my PR, please have a look. Thanks.

@weierophinney
Copy link
Member

@webimpress I was working on similar changes, after reviewing this and zend-servicemanager-di.

I took your original changeset and was able to apply it against zend-servicemanager-di (with attribution, even; I used the .patch from this PR, edited it, and applied it to that component, which gives you credit for the commit), which fixed the related issue in that component. I've now opened #158, which:

  • adds zend-servicemanager-di to the component requirements
  • updates 4 of the DI-related factories to simply extend from the zend-servicemanager-di variants
  • marks all 5 of the DI-related factories as deprecated via annotation

This approach is completely BC; the four I created as extensions were returning artifacts from the Zend\ServiceManager\Di namespace already, and the fifth now returns one of these extending classes, preserving inheritance.

Thanks for raising the issue, and for your patches!

weierophinney added a commit that referenced this pull request Jun 11, 2016
weierophinney added a commit that referenced this pull request Jun 11, 2016
@michalbundyra
Copy link
Member Author

@weierophinney thank you, it's much better now, thanks :)

@michalbundyra michalbundyra deleted the hotfix/148 branch June 13, 2016 08:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants