-
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
Reorganize abstracts & interfaces #6
Conversation
- Reorganized abstract classes and interfaces into `Contracts/` directories. - Switched away from `StellarWP\Schema\Container` in favor of using `lucatume\DI52\App` and `lucatume\DI52\Container`.
@borkweb Rather than introducing a new package and container in multiple packages, I'm inclined to make it possible within package configuration for the implementing plugin to provide its own PSR-11 compatible service container instance — which is then used by the package. What do you think? |
Man, that PSR-11 interface sucks more than I realized. It's way too limited. It doesn't even have the ability to set items within the container, much less singletons. We may consider making our own StellarWP/Container package that's, at the very least, our own interface. |
@JasonTheAdams - I agree that it'd be great to provide a way to inject a container. That being said, I'd love this library to provide a sensible default for those that do not have the desire or knowledge of how to provide a container. We definitely need to have a more lengthy conversation around this topic with some more folks. |
I guess that depends on the assumptions we're willing to make. If we include this as a default for every package that uses a Container, then that's increasing the size of that package (and plugin) by ~1 MB/plugin — assuming this PR gets merged and it's not 50 MB: lucatume/di52#39 😬 At this point, I'd be willing to assume the implementing plugin will provide its own Container to reduce size and complexity per package, until such a time as folks complain and make an Issue. Even then I'd still prefer to have a separate Container package we can point folks towards to implement and provide. |
Switching this back to draft mode because #14 rethinks some of the foundational elements that these changes build on. |
Alrighty, with the addition of a |
Contracts/
directories. (I like that approach from Give)StellarWP\Schema\Container
in favor of usinglucatume\DI52\App
andlucatume\DI52\Container
.This builds on top of #5 with the intent of being part of
1.1.0
.Addresses: #8 and #9