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

Feature : Dependency injection with dagger/hilt #1106

Open
epicadk opened this issue Mar 3, 2021 · 9 comments
Open

Feature : Dependency injection with dagger/hilt #1106

epicadk opened this issue Mar 3, 2021 · 9 comments
Assignees
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Maintenance Repository maintenance.

Comments

@epicadk
Copy link
Member

epicadk commented Mar 3, 2021

Is your feature request related to a problem? Please describe.

As someone who wants to write tests for the app dependency injection with hilt/dagger will make it much easier as explained here. It is also listed under best practices according to the docs.

Describe the solution you'd like

Perform dependency injection with dagger/hilt

Describe alternatives you've considered

Hilt is still in beta however hilt seems like the way to go in the future. Dagger has a huge community and is used by a lot of huge projects and has a stable release.

@epicadk epicadk added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Available Issue was approved and available to claim or abandoned for over 3 days. Type: Maintenance Repository maintenance. labels Mar 3, 2021
@epicadk
Copy link
Member Author

epicadk commented Mar 3, 2021

@isabelcosta opened the issue as suggested in yesterdays open session. I can take this up : ).

@CodeChamp-SS
Copy link
Contributor

@epicadk I can take up this issue if it's approved and available, I've also done research on implementing hilt in the app during OSH :-)

@epicadk
Copy link
Member Author

epicadk commented Mar 3, 2021

@epicadk I can take up this issue if it's approved and available, I've also done research on implementing hilt in the app during OSH :-)

As I also wanted to work on this it is kind of a conflict of interest so I'll leave the decision with the maintainers @vj-codes @isabelcosta

@justdvnsh
Copy link
Contributor

Well, I was also wondering to take up this issue @epicadk.

@kartikeysaran
Copy link
Contributor

This will be a nice feature to add ! @epicadk 🙌

@vj-codes
Copy link
Member

vj-codes commented Mar 4, 2021

Ideally, the issue is assigned to the contributor who claimed it first , ie @epicadk
But since this is a huge issue and will probably take more time and refactoring maybe it can be divided into parts
I'm not much aware but it was suggested that hilt is a better option than dagger long back
So is it possible to divide this issue? @epicadk

@epicadk
Copy link
Member Author

epicadk commented Mar 4, 2021

Ideally, the issue is assigned to the contributor who claimed it first , ie @epicadk
But since this is a huge issue and will probably take more time and refactoring maybe it can be divided into parts
I'm not much aware but it was suggested that hilt is a better option than dagger long back
So is it possible to divide this issue? @epicadk

It would be great if we can divide it into parts however I am not sure how we can do that. Even if we do manage to do that I'm not sure we'll be able to work on these parts simultaneously. Does anyone have any suggestions? @justdvnsh @CodeChamp-SS

P.s hilt has only recently launched its beta so if it wouldn't have been a good idea to use hilt before as alpha APIs are constantly changing and aren't tested as well as the beta is. The beta version that was launched around 7-8 days ago will not undergo and changes in the api and only bug fixes will be made to tbe library if any.

@justdvnsh
Copy link
Contributor

justdvnsh commented Mar 4, 2021

@vj-codes Undoubtedly, dagger hilt is the way to go, since it makes DI and testing a breeze all while maintaining the security issues. But I agree with @epicadk, I don't think that It could be divided into multiple parts. Most of what we can do, it to create an issue, for simply adding the library, and then other issue for refactoring the whole app. But that would be cumbersome. Since, once we start migrating the app to dagger hilt, we would have to migrate the app completely, since the features will start breaking. I am also out of ideas if we'd be able to break this issue into multiple pieces.
So, personally I think it would be best to just completely migrate the app into dagger-hilt. Moreover, @epicadk would be able to complete it within few days. I'd take up #1072 if it becomes available.

@CodeChamp-SS
Copy link
Contributor

@vj-codes I agree with @epicadk and @justdvnsh that it would't be easy to divide the issue into pieces. I was eager to work on this issue as I had also done research on this topic in OSH and had looked into implementing hilt in the app, but it's ok if the issue has to be assigned to one who claimed it first :-)

@vj-codes vj-codes removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Maintenance Repository maintenance.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants