Add PSR-11 support #450
Add PSR-11 support #450
Conversation
composer.json
Outdated
| "psr/http-message": "^1.0", | ||
| "psr/container": "^1.0", |
There was a problem hiding this comment.
Can we have here alphabetical order, so psr/container before psr/http-message, please?
There was a problem hiding this comment.
We can make this fully BC by requiring:
"container-interop/container-interop": "^1.2""psr/container": "^1.0"
That way, if folks are still type-hinting on container-interop interfaces, their code will continue to work. We can message then that we'll be removing support for container-interop in the next major version (v3).
One reason we can't go full-on with psr/container quite yet is because none of the supported containers currently support it. Until there are versions that do, we'd actually make Expressive useless!
There was a problem hiding this comment.
Hm... we can't change namespaces in expressive 1.X to Psr\Container ... it will be BC break.
I think we can do it with v2 (as it is not yet released), because container-interop 1.2 extends psr-container, so for example ServiceManager with container-interop 1.2 is actually PSR-11 compatible.
There was a problem hiding this comment.
Alternately, we could have a conflict section:
"require": {
"psr/container": "^1.0"
},
"conflict: {
"container-interop/container-interop": "<1.2.0"
}Since the various container implementations we support all require container-interop currently, and version 1.2.0 of container-interop extends the various psr/container interfaces, then an update would grab that version, and the various typehints we use would work.
There was a problem hiding this comment.
@xtreamwayz Can you re-target this PR at the develop branch, and add the conflict statement I've suggested above? If so, I'll give it a spin, and see how my various applications work with it.
There was a problem hiding this comment.
@weierophinney I was suggesting adding "conflict" in my comment below ;-)
#450 (comment)
There was a problem hiding this comment.
@webimpress Don't worry, I have seen it :)
There was a problem hiding this comment.
@xtreamwayz Please sort packages in alphabetical order as I requested in first comment. Thx!
|
I can see that tests fails also with lowest dependencies, maybe we should add: "conflict": {
"container-interop/container-interop": "<1.2"
},into |
|
Isn't this PR a BC Break? We have now typehints for different
classes/interfaces, so if somebody extends one of our classes needs to
update his source as well. Am I wrong?
I was thinking the same after I created the PR. I'll leave it as is for the
discussion. If needed I'll revise on develop.
I can see that tests fails also with lowest dependencies, maybe we should
add...
That might fix the issue.
Op wo 15 feb. 2017 09:41 schreef Michał Bundyra <[email protected]>:
… I can see that tests fails also with lowest dependencies, maybe we should
add:
"conflict": {
"container-interop/container-interop": "<1.2"
},
into composer.json?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#450 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJDr8g3XjcW3ivMPWMGM-X9faVKKrtxJks5rcrpVgaJpZM4MBVxl>
.
|
|
@weierophinney Rebased on develop. The docs need to be checked again. |
|
Labeled as BC break, as anybody extending the various classes where signatures have typehints against container-interop would break — this is, of course, also why it's put in the 2.0.0 milestone. |
Add PSR-11 support Conflicts: composer.lock
|
Thanks, @xtreamwayz! |
Replaces:
Interop\Container\ContainerInterface=>Psr\Container\ContainerInterfaceInterop\Container\Exception\ContainerException=>Psr\Container\ContainerExceptionInterfaceInterop\Container\Exception\NotFoundException=>Psr\Container\NotFoundExceptionInterfaceUpdates all container-interop references in the docs.