-
Notifications
You must be signed in to change notification settings - Fork 2
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 to use psr-11 #2
Conversation
@@ -46,7 +46,7 @@ integrating Auryn with third-party libraries that use *container-interop*, such | |||
as [zend-expressive](https://github.com/zendframework/zend-expressive). | |||
|
|||
The implementation in this repository takes a small amount of liberty with this | |||
passage from [Section 1.1](https://github.com/container-interop/container-interop/blob/master/docs/ContainerInterface.md#11-basics) | |||
passage from [Section 1.1](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-11-container.md#11-basics) |
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.
Hi @elazar ,
I think probably we can get rid of this section, for the current get and has doesn't have the problem mentioned as in container-interop. Let me know what you think.
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.
@harikt I'm not sure I follow. The same quote exists in both documents. Can you elaborate?
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.
Hi @elazar ,
The delegate lookup feature is not currently in PSR-11
, but is only present in container-interop
. So we don't need to worry about the has for the current time.
See discussion over laravel/ideas#803 (comment) .
Let me know if that helps! .
Thank you.
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.
@harikt I think I see what you're getting at. I'm open to suggestions for amending or removing the cited passage.
I think using something like northwoods/container is more appropriate, as it wraps Auryn rather than extending it. This is more consistent with SOLID principles. |
@shadowhand Seems sort of similar to elazar/auryn-configuration ? |
@elazar it certainly has some crossover. And also with https://packagist.org/packages/equip/config. |
Closing in favor of #4. |
Hi @elazar ,
I was looking at Auryn and came across rdlowrey/auryn#77 and container-interop/container-interop#75 .
As psr-11 is already there may be we could update the library to just depend on psr/container. Even though without this PR it is satisfied via container-interop . May be we can bump this up ?
Thank you.