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

Add the option to resolve dependencies by name #39

Merged
merged 4 commits into from
Apr 6, 2020

Conversation

brototyp
Copy link
Contributor

@brototyp brototyp commented Apr 5, 2020

Implements #8

This PR add functionality to resolve dependencies by name or in this case tag.
In order to do so, I changed the following:

A component no longer has a tag: String but an let identifier: AnyHashable. This identifier is used to reference the Component as well as the instance in the Container (ComponentStack and InstanceStack). I also added a ComponentIdentifier struct that uses the Type of the factory as well as an optional tag to calculate the hashValue. So instead of a String of the Type, we now use the hashValue of the type to identify a Component or an instance in our Container.

Next I added an optional tag: AnyHashable parameter to all functions that are used to register or resolve dependencies.

I didn't write any documentation in the README yet since I first wanted to get some feedback about this first. I am looking forward to hear ideas on how this could be improved.

@benjohnde
Copy link
Member

Oh my gosh that's amazing! @brototyp I am going to review your PR tomorrow. Thank you so much for your work! I am so happy right now to see some interest in that tiny DI library :)

Copy link
Member

@benjohnde benjohnde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like the usage of AnyHashable and also the new ComponentIdentifier. Thank you very much for your contribution!

@benjohnde benjohnde merged commit ec4e244 into Liftric:master Apr 6, 2020
@brototyp brototyp deleted the feature/8_resolving_by_name branch April 6, 2020 17:44
@brototyp
Copy link
Contributor Author

brototyp commented Apr 6, 2020

Wohoo. Awesome! Thanks for your fast review!

@benjohnde
Copy link
Member

Ah damn, I merged it a bit too fast, as you mentioned the missing documentation in the README.md. Well @brototyp, feel free to open another PR. I am going to give #22 another shot. After that I will bump the version to 1.6.0 and release your contribution!

@brototyp
Copy link
Contributor Author

brototyp commented Apr 6, 2020

Ah damn, I merged it a bit too fast, as you mentioned the missing documentation in the README.md. Well @brototyp, feel free to open another PR. I am going to give #22 another shot. After that I will bump the version to 1.6.0 and release your contribution!

Oh yeah. I totally forgot as well. Looks like we both were a little to excited about it 😊
I'll try to write a first draft of the README and will create a PR in that regard.

@brototyp
Copy link
Contributor Author

Hey @benjohnde, just a gentle ping from my side. Is there any way we can get the release train rollin'? Either straight as it is, or with #22? (Also: Do you need any support on #22? Maybe some brainstorming and bouncing off ideas?)

@benjohnde
Copy link
Member

benjohnde commented Aug 26, 2020

I am ultimately sorry for having merged your contribution and not releasing it. I totally love the way you implemented the resolving dependencies by name feature. I am going to finalise the release tomorrow @brototyp 🎉🚀.

@brototyp
Copy link
Contributor Author

Don't worry about it. I know that it's hard to follow up on such side projects.

@benjohnde
Copy link
Member

I just finished the release, you are free to use the new version 1.6.0. Podspec was also pushed to Cocoapods 🎉 !

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