-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Async init #19
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
feat: Async init #19
Conversation
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.
Well, I apologize for request changes, but I just wanna you double check the solution as well. There are many similarities in all those typealiases for "factories" but some of the types are not very understandable - although it's async copy paste of existing.
I know we're adding new "feature" without breaking changes, at the same time we created async DI for exploring swift6 complete concurrency check really fast way, therefore I'm for double check.
…ependency-injection into feat/rob-async-init
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.
Very handy! I have just a few documentation and tests comments.
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.
I just looked at it briefly and left one thought. I'll do more throught review once I have more time. 😊
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.
@robha141 Kudos. Impressive work. Please consider the change in the folder structure - maybe we can put Sync and Async variants of code into separate subfolders. What do you think?
And maybe silly question. If I have the same dependency that I need in both sync and async contexts, should I have two registrations? Can we somehow unify the interface as @cernym46 suggested?
Sources/Protocols/Registration/AsyncDependencyRegistering.swift
Outdated
Show resolved
Hide resolved
Co-authored-by: Filip Haškovec <[email protected]>
Co-authored-by: Filip Haškovec <[email protected]>
…cy-injection into feat/rob-async-init
I made the changes in the structure. Those changes are rather cosmetic but they allow to divide of synchronous, asynchronous, and common parts of the code. |
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.
Great job! I didn't find anything that would have effect on the usage. Just few naming/documentation notes and one nitpicking edgecase.
Co-authored-by: Filip Haškovec <[email protected]>
Co-authored-by: Filip Haškovec <[email protected]>
…cy-injection into feat/rob-async-init
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.
🍚
Added new AsyncContainer, with async registration and resolution of dependencies.