Marcello Galhardo
11/04/2021, 3:19 PMsingle
, factory
, viewModel
) to reduce the new
. I think that is OK because I don’t foresee many new binding definitions to be created in the future. That is how it would look like (see the picture for comparison):
factory(::RecoveryReminderViewModel)
Locally is already working, there is only one points that still concerns me: the namespace / auto complete might be a little polluted (see picture below). What do you think? If you don’t see a problem I think we can move forward and I can do PRs with it next days. Maybe someone else has other opinions here too. 🙂Marcello Galhardo
11/04/2021, 3:55 PMviewModel
(as both are extension functions). 🤔
Overload resolution ambiguity. All these functions match.
Marcello Galhardo
11/04/2021, 4:02 PMMarcello Galhardo
11/04/2021, 4:04 PMautoFactory
, autoViewModel
or something like that? (auto is not a good eaxmple, but I don’t know what would be an ideal word).Marcello Galhardo
11/04/2021, 4:28 PMbindFactory
, bind would be nice but I guess is out of the table now that Koin has a bind API already. Maybe resolveFactory
?arnaud.giuliani
11/04/2021, 5:08 PMarnaud.giuliani
11/04/2021, 5:10 PMarnaud.giuliani
11/04/2021, 5:11 PMarnaud.giuliani
11/04/2021, 5:12 PMModule.factory()
, the IDE will autocomplete with all available APIsarnaud.giuliani
11/04/2021, 5:13 PMresolve
implies something already “technical” for me ... let’s find some good ideasarnaud.giuliani
11/04/2021, 5:13 PMnewFactory(::MyComponent)
arnaud.giuliani
11/04/2021, 5:13 PMarnaud.giuliani
11/04/2021, 5:13 PMfactoryOf(::MyComponent)
Marcello Galhardo
11/04/2021, 5:15 PMfactory
as factoryOf
might be a candidate for resolution. Additionally, of
in Kotlin implies a list of somesort… 🤔Marcello Galhardo
11/04/2021, 5:15 PMMarcello Galhardo
11/04/2021, 5:16 PMnewFactory
, newViewModel
and newSingle
are the best options. 🤔arnaud.giuliani
11/04/2021, 5:17 PMMarcello Galhardo
11/04/2021, 5:17 PMarnaud.giuliani
11/04/2021, 5:17 PMMarcello Galhardo
11/04/2021, 5:17 PMMarcello Galhardo
11/04/2021, 5:45 PMmake
might be a good option. makeFactory
, makeViewModel
and makeSingle
. It doesn’t match any other Koin API, and it is not as common as new
in the JVM ecosystem. new
can also be renamed to make
and keep same concept. We can call it “make API” to distinguish it. And it is also short. 🙂arnaud.giuliani
11/04/2021, 5:57 PMfactoryOf
can be autocomplete in an easiest way, it come just after factory
arnaud.giuliani
11/04/2021, 5:58 PMMarcello Galhardo
11/05/2021, 12:36 AMMarcello Galhardo
11/05/2021, 9:36 AMviewModelOf
feels a little like it will return something but so far is the best option IMO - reads well, auto complete is very much good, feels natural with other Kotlin APIs (e.g., mutableStateOf
or listOf
). I updated the PR using this approach for now, so feel free to clone and give it a try. 🙂arnaud.giuliani
11/05/2021, 10:38 AMarnaud.giuliani
11/12/2021, 8:27 AM*Of
. But I’m struggling with the qualifier overload problem 😕Marcello Galhardo
11/12/2021, 6:02 PMarnaud.giuliani
11/15/2021, 9:59 AMarnaud.giuliani
11/15/2021, 9:59 AMMarcello Galhardo
11/15/2021, 3:07 PMnew
method and completely drop the current Pull Request as it is more composable as a side effect of having some extra ceremony (my initial proposal).
2. We keep the two overloads for each method, as it allow us to maintain consistency with our previous API. For our API consumers, that removes boilerplate and they don’t need to know about the manual overloading (I just need to revert my previous commit).
3. We keep the current order and breaking consistency. That is a little worse in API perspective but there is some advantages: Kotlin won’t suggest an anonymous lambda like it does if the constructor is in the end and you can still use named parameters to keep the order you prefer.
All those options have their advantage and disadvantages - from the current work we already did and the fact we are trying to reduce ceremony, option 2 sounds like the best one. We put the ceremony in us in favor to provide a better API for our users - also, I don’t expect those methods to change for a long-long time...arnaud.giuliani
11/16/2021, 7:05 AMviewModelOf
? 🤔Marcello Galhardo
11/16/2021, 9:22 AMfun viewModelOf(constructor)
fun viewModelOf(qualifier, constructor)
Manually writing the two signatures (with and without the qualifier) works fine. I changed it only because of your comment in the PR:
concerning qualifier overload signature, I was thinking perhaps to have the constructor first and then nullable qualifier.
If that option looks good I can change my PR to include it. 🤔
arnaud.giuliani
11/16/2021, 9:39 AMviewModelOf(::constructor)
and viewModelOf(qualifier, ::constructor)
there is not signature conflict then? (sorry, just to be sure 😛 )arnaud.giuliani
11/16/2021, 9:40 AMIf that option looks good I can change my PR to include it. 🤔
arnaud.giuliani
11/16/2021, 9:40 AMarnaud.giuliani
11/16/2021, 9:40 AMMarcello Galhardo
11/16/2021, 1:03 PM3.2.0
. We are adding functionality in a backward compatible manner. Patches are only for bug-fixes which isn’t the case here. The new features are really nice but it isn't big enough for a major. If we have some luck, we can include the Module Grouping
in version 3.2.0
, otherwise it would need to go to 3.3.0
- it is your call. 🤔
I'm not sure how you normally announce releases but I think those features could become a short Twitter Thread or Blog Post - as they will reduce tons of boilerplate. (:arnaud.giuliani
11/16/2021, 4:38 PMarnaud.giuliani
11/16/2021, 4:39 PMnew
operator is in 3.1.x … I can check how to move it 🤔arnaud.giuliani
11/16/2021, 4:39 PMarnaud.giuliani
11/16/2021, 4:40 PMMarcello Galhardo
11/16/2021, 10:22 PM3.1.x
- isn’t it easier to simple to bump it and start a new release cycle for 3.2.x
and so on? We can patch 3.1 inside 3.2 as they are backward compatible... 🤔
If for some reason that is not possible, we don’t necessary need to change a lot - we could add an experimental
contract annotation to the new
method so developers should not use it - and not communicate it officially right now as an in test feature - while only the 3.2.x
release will have the annotation removed.arnaud.giuliani
11/17/2021, 7:32 PMarnaud.giuliani
11/17/2021, 7:32 PMMarcello Galhardo
11/17/2021, 8:31 PMarnaud.giuliani
11/18/2021, 1:04 PMarnaud.giuliani
11/18/2021, 1:05 PMMarcello Galhardo
11/18/2021, 2:27 PMMarcello Galhardo
11/18/2021, 5:37 PMarnaud.giuliani
11/19/2021, 10:08 AMarnaud.giuliani
11/19/2021, 10:19 AMmain
arnaud.giuliani
11/19/2021, 10:19 AMmaster
arnaud.giuliani
11/19/2021, 10:19 AMMarcello Galhardo
11/19/2021, 1:00 PMarnaud.giuliani
11/19/2021, 1:19 PMMarcello Galhardo
12/08/2021, 10:13 AMI can help to setup some doc around
arnaud.giuliani
12/09/2021, 9:11 AMMarcello Galhardo
12/09/2021, 12:32 PM