search

badoo / MVIKotlin

Extendable MVI framework for Kotlin Multiplatform with powerful debugging tools (logging and time travel), inspired by Badoo MVICore library
https://arkivanov.github.io/MVIKotlin
Apache License 2.0
825 stars 66 forks source link

Update KDocs in CoroutineExecutor #312

Open shynline opened 2 years ago

shynline commented 2 years ago

https://github.com/arkivanov/MVIKotlin/blob/4bad89cb0b5aee04e3e9fa04d90bd4fbbc37900e/mvikotlin-extensions-coroutines/src/commonMain/kotlin/com/arkivanov/mvikotlin/extensions/coroutines/CoroutineExecutor.kt#L33-L35

https://github.com/arkivanov/MVIKotlin/blob/4bad89cb0b5aee04e3e9fa04d90bd4fbbc37900e/mvikotlin-extensions-coroutines/src/commonMain/kotlin/com/arkivanov/mvikotlin/extensions/coroutines/CoroutineExecutor.kt#L48-L50

arkivanov commented 2 years ago

Hello, could you please elaborate? What do you think is not right?

arkivanov commented 2 years ago

Now it's clear what you mean. I will copy my message from Kotlin Slack here for transparency.

Both executeAction and executeIntent were suspending functions a while ago, but it was an explicit decision to make them normal.

There is scope properly available which you can use to launch coroutines when needed. You can check samples: https://github.com/arkivanov/MVIKotlin/blob/master/sample/todo-coroutines/src/commonMain/kotlin/com/arkivanov/mvikotlin/sample/todo/coroutines/store/TodoListStoreFactory.kt

The reason for this change is because now you are able to process intents and actions synchronously in the same call stack as they were dispatched.

shynline commented 2 years ago

Oh, I see. I honestly didn't know about the decision and also, comments for those functions didn't help since they're mentioning suspend function still. thanks

arkivanov commented 2 years ago

Good point! I think we need to update KDocs instead.

arkivanov commented 2 years ago

Moved to https://github.com/arkivanov/MVIKotlin/issues/2