Closed joramnv closed 4 years ago
See conversation in Slack for more context: https://kotlinlang.slack.com/archives/C5UPMM0A0/p1599853983245600
Hi @joramnv :wave: Thank you so much for your contribution!! :raised_hands:
It seems the change impacts on Arrow Fx. I think it won't be easy to find the reason because I didn't see real changes but additions here.
The failed CI checks test all the Arrow libraries with this pull request. In order to reproduce the error, please, run this command:
./gradlew buildWithLocalDeps
Instead of using Arrow dependencies from an external repository, that command will install Arrow dependencies in your local repository. You'll see something like:
arrow-core-lib [publishToMavenLocal]
...
arrow-ui-lib [publishToMavenLocal]
...
arrow-fx-lib [publishToMavenLocal]
...
(error)
I hope it's useful to detect the reason of the error. Please, let us know if we can help you :raised_hands:
Hi @joramnv 👋 Thank you so much for your contribution!! 🙌
It seems the change impacts on Arrow Fx. I think it won't be easy to find the reason because I didn't see real changes but additions here.
The failed CI checks test all the Arrow libraries with this pull request. In order to reproduce the error, please, run this command:
./gradlew buildWithLocalDeps
Instead of using Arrow dependencies from an external repository, that command will install Arrow dependencies in your local repository. You'll see something like:
arrow-core-lib [publishToMavenLocal] ... arrow-ui-lib [publishToMavenLocal] ... arrow-fx-lib [publishToMavenLocal] ... (error)
I hope it's useful to detect the reason of the error. Please, let us know if we can help you 🙌
Thanks for the pointer. Sadly, the buildWithLocalDeps
task results in:
> Task :checkoutOrchestrator FAILED
Clone arrow repository in /Users/joram/projects/arrow ...
Cloning into 'arrow'...
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
FAILURE: Build failed with an exception.
when (val res2 = Either.catch { scope.open(res.isInterruptible) }) {
(line 238 in arrow-fx-coroutines/src/main/kotlin/arrow/fx/coroutines/stream/Compiler.kt
) does not seem to be happy with Either.catch
being an inline function.
I see two possibilities; revert Either.catch
back to not being inlined in this PR or change the implementation in the arrow-fx-coroutines/src/main/kotlin/arrow/fx/coroutines/stream/Compiler.kt
(to use a separate not inlined local version of Either.catch
or to make it work with the inline function).
Hi @joramnv , I think inlining the body in the stream compiler is the best option since Either.catch is heavily used and the Compiler impl is private. If we find a small repro for this outside Arrow we should consider reporting to the Kotlin tracker if isolated. Thanks!
Thanks for the feedback @joramnv !! I'll add support for https as well :+1:
Hi @joramnv , I think inlining the body in the stream compiler is the best option since Either.catch is heavily used and the Compiler impl is private. If we find a small repro for this outside Arrow we should consider reporting to the Kotlin tracker if isolated. Thanks!
Created a new PR for this: https://github.com/arrow-kt/arrow-fx/pull/288
Same implementation as the handle function in https://github.com/sparetimedevs/pofpaf/blob/master/src/main/kotlin/com/sparetimedevs/pofpaf/handler/Handler.kt.
I did rename the function to
resolve
, because I like it better thanhandle
in combination withEither
;Either.resolve
. But I am open to naming suggestions.Concerning the tests. I wrote the original tests using Kotest. I quickly ported the tests to KotlinTest and they are not optimal. I did see that there is an open PR to upgrade the project to Kotest (https://github.com/arrow-kt/arrow-core/pull/188). If that is merged soon, there won't be a need for the
runBlocking
workaround.