Improve Arrow Fx Coroutines docs for release #248
Conversation
arrow-fx-coroutines/src/main/kotlin/arrow/fx/coroutines/Bracket.kt
Outdated
Show resolved
Hide resolved
| * Reason being, we cannot [release] what we did not `acquire` first. Same reason we cannot call [use]. | ||
| * If it is successful we pass the result to stage 2 [use]. | ||
| * | ||
| * 2. Resource consumption is like any other `suspend` effect. The key difference here is that it's wired in such a way that |
There was a problem hiding this comment.
Excellent docs and clarifications!
There was a problem hiding this comment.
Should we repeat this in bracket, I am unsure how we can improve the documentation of these top-level functions.
API docs are a very low profile and have little to no styling. I'm not sure how or if we need to improve our API docs, or if we should promote certain functions in a different manner. parMapN, parTupledN, raceN.
| * } | ||
| * | ||
| * suspend fun main(): Unit { | ||
| * val fiber = ForkConnected { |
There was a problem hiding this comment.
I'm not clear about the name ForkConnected. Could you elaborate a bit about why the Connected part?
There was a problem hiding this comment.
I added docs for ForkConnected with an example after this comment :D link
Reasoning for new ForkConnected and ForkScoped functions
In Arrow Fx IO we only had fork which launches a Fiber without any cancellation connection control. Meaning you always leak unless you launch a Fiber inside Resource or bracketCase as the acquire step.
This makes fork quite complicated and easily results in cancellation bugs etc. Those semantics of IO's fork is now an combinator called `ForkAndForget.
Some more info here. We should probably also document these, but this comes back to trying to figure out a way to add higher-level documentation to top-level functions besides function docs. Or do you think we should include all of that there?
…t.kt Co-authored-by: Raúl Raja Martínez <raulraja@gmail.com>
|
@raulraja Thanks for the review! Did I address all your comments? Do you have any feedback on my two questions (checkboxes)? |
| isDaemon = true | ||
| } | ||
| }.asCoroutineContext() | ||
|
|
There was a problem hiding this comment.
Awesome @nomisRev !! I think KLint is complaining about this extra line!
arrow-fx-coroutines/src/main/kotlin/arrow/fx/coroutines/Bracket.kt
Outdated
Show resolved
Hide resolved
| @@ -14,6 +14,32 @@ import kotlin.coroutines.intrinsics.COROUTINE_SUSPENDED | |||
| * The winner of the race cancels the other participants, | |||
| * cancelling the operation cancels all participants. | |||
There was a problem hiding this comment.
What if a participant is uncancellable ?
| @@ -209,6 +212,61 @@ sealed class Resource<out A> { | |||
| fun <A> defer(f: suspend () -> Resource<A>): Resource<A> = | |||
| Resource.Defer(f) | |||
|
|
|||
| /** | |||
| * Creates a single threaded [CoroutineContext] as a [Resource]. | |||
There was a problem hiding this comment.
Please add detail (release part) : e.g. specify that the submitted tasks will be executed before terminating
| Resource(f) { s -> s.shutdown() }.map(ExecutorService::asCoroutineContext) | ||
|
|
||
| /** | ||
| * Creates a single threaded [CoroutineContext] as a [Resource]. |
There was a problem hiding this comment.
Please add detail (release part) : e.g. specify that the submitted tasks will be executed before terminating
| @@ -6,6 +6,28 @@ import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn | |||
| /** | |||
| * Checks for cancellation, | |||
| * when cancellation occurs this coroutine will suspend indefinitely and never continue. | |||
There was a problem hiding this comment.
The example is clear ... but, the textual explanation does not seem sufficient to understand the purpose of cancelBoundary
There was a problem hiding this comment.
What about the following instead? I found this terribly hard to document.
"Inserts a cancellable boundary — it checks for the cancellation status of the suspend-loop and does not allow the continuation to keep executing in the case cancellation happened."
There was a problem hiding this comment.
Maybe:
"In a cancelable environment, we need to add mechanisms to react when a cancellation is triggered : a cancel boundary inserts checking point in suspendable methods. At runtime, a cancel boundary checks for the cancellation status; and, it does not allow the continuation of task to keep executing in the case a cancellation was triggered.
Useful, for example, to cancel the continuation of a loop, as shown in this code snippet:"
Or, something like that
There was a problem hiding this comment.
Thank you @PhBastiani! That is a much better improvement over what I did 😅
|
About:
Let's try the new Dokka which introduces a modern HTML format!! |
|
@PhBastiani Thanks for the amazing review and taking the time to go through the PR! 👍 |
Another iteration over the docs.
This PR goes together with arrow-kt/arrow-site#64 which updates the sidebar of the Arrow Fx website to promote Arrow Fx Coroutines for the next version.
Quickstart section
Updated the second page from the Arrow Fx website Quickstart to Arrow Fx Coroutines, this leaves the 3rd page which talks about tagless effects.
This could become a page that in more detail explains the integrations and cancellation in the standard library, etc.
A fourth page could be added to the quickstart which covers the
IOvssuspendsection currently in the README.MD of Arrow Fx Coroutines. It could also include a small migration guide fromIOtosuspendas today was asked on SlackData type section (non-blockers for PR)
CircuitBreaker only has API documentation. It was inspired by Monix's CircuitBreaker which in turn was inspired by Akka's CircuitBreaker. Monix's seems to overlap with Akka's docs, so perhaps with the correct credits we can do the same. link.
API docs are a very low profile and have little to no styling. I'm not sure how or if we need to improve our API docs, or if we should promote certain functions in a different manner. parMapN, parTupledN, raceN.