anitab-org / mentorship-android

Mentorship System is an application that matches women in tech to mentor each other, on career development, through 1:1 relations during a certain period of time. This is the Android application of this project.
https://github.com/anitab-org/mentorship-android/raw/apk/app-debug.apk
GNU General Public License v3.0
167 stars 378 forks source link

Feature : Dependency injection with dagger/hilt #1106

Open epicadk opened 3 years ago

epicadk commented 3 years ago

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 commented 3 years ago

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

CodeChamp-SS commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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

kartikeysaran commented 3 years ago

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

vj-codes commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 :-)