Skip to content
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

ContainerProtocol.register returns self #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tdg5
Copy link

@tdg5 tdg5 commented Feb 18, 2024

Y'all may have a reason for not defining the return type of ContainerProtocol.register, but if so, I didn't pick up on it.

Here I propose making ContainerProtocol.register return Self. This matches what Container.register does and allows a chaining of registrations of like

container.register(...).register(...).register(...)

Honestly, I don't care and am equally happy with None, but a tighter bound than Any seems beneficial to maintaining portability across container implementations.

@RobertoPrevato
Copy link
Member

Hi @tdg5
Sorry for replying so late. The reason why I didn't specify return self in the ContainerProtocol is only to keep simple the protocol that can be used to replace rodi with alternative implementations of DI in applications like BlackSheep apps.
But I like your idea and I am considering to merge. Thank You!

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

Successfully merging this pull request may close these issues.

None yet

2 participants