Feature/test #9

JesusMcCloud · 2024-10-10T07:00:30Z

migrate to dokka 2 for documentation
multi-module project setup
introduce kmmresult-test, enabling
- result should succeed
- result shouldNot succeed
- result shouldSucceedWith expectedValue
remove Arrow dependency and import arrow's list of Fatal exceptions directly into our code
Introduce [Result.nonFatalOrThrow] to mimic KmmResult's non-fatal-only behaviour, but without the object instantiation overhead

iaik-jheher · 2024-10-11T08:23:27Z

would result.shouldSucceed() which returns the contained value (or throws) be useful?

JesusMcCloud · 2024-10-12T05:28:02Z

done

iaik-jheher · 2024-10-14T10:11:12Z

maybe a more "natural" way to avoid forgetting .nonFatalOrThrow internally... runCatchingNonFatal? (it's a mouthful, maybe find a shorter name?)

iaik-jheher · 2024-10-14T10:14:30Z

so just a shorthand for runCatching { ... }.nonFatalOrThrow()

nodh · 2024-10-14T10:21:33Z

buildSrc/settings.gradle

@@ -0,0 +1 @@
+rootProject.name = 'backend_buildSrc'


why backend?

overlooked this. renamed

nodh · 2024-10-14T10:21:54Z

buildSrc/src/main/kotlin/Versions.kt

@@ -0,0 +1,29 @@
+object VersionsBackend {


Why backend?

totally forgot to remove the actual sources. in the buildSrc. buildSrc is needed becasue dokka only works from plugins (such as our conventions, or buildSrc) for multi-module projects)

nodh · 2024-10-14T10:22:20Z

buildSrc/src/main/kotlin/Versions.kt

+    const val attestation = "2.1.3"
+    const val jose = "9.25.6"
+
+    object spring {


why do we need spring dependency declarations here?

nodh · 2024-10-14T10:22:38Z

gradle.properties

Non-snapshot version on development?

i really should rename the default branch to main and create a proper development branch. Will do after squashing and merging this.

kmmresult/src/commonTest/kotlin/KmmResultTest.kt

JesusMcCloud · 2024-10-16T06:23:25Z

@iaik-jheher

maybe a more "natural" way to avoid forgetting .nonFatalOrThrow internally... runCatchingNonFatal? (it's a mouthful, maybe find a shorter name?)

My issue with this proposal is that it makes it easy to accidentally expose a Result, when you really want a KmmResult for your public API. After all, you'd then have two functions:

catching
nonFatalCatching or runCatchinNonFatal

Both will be rethrowing fatal exceptions, but only has it in its name. I don't see how to solve that issue, because even if we rename catching (which we cannot, because it would break evrything)m it won't change the fact that we'd end up with a confusing API.
Result is borked and we can't fix it. We can only offer a band-aid and .nonFatalOrThrow() is pretty expressive in that repsect.

iaik-jheher · 2024-10-16T11:13:36Z

i thought the intent of nonFatalOrThrow was that we could "safely" use Result internally to avoid overhead; did I misunderstand this?

JesusMcCloud · 2024-10-16T11:22:49Z

i thought the intent of nonFatalOrThrow was that we could "safely" use Result internally to avoid overhead; did I misunderstand this?

Yes it is and this is what it does. What is the issue?

iaik-jheher · 2024-10-16T11:24:11Z

I don't think using it internally is safe if forgetting nonFatalOrThrow is an undetectable footgun...

iaik-jheher · 2024-10-16T11:25:11Z

(That was the thought process behind my suggestion for a runCatching replacement)

JesusMcCloud · 2024-10-16T11:26:25Z

Fair point, but how to not make the API confusing?

JesusMcCloud · 2024-10-16T11:28:13Z

thinking about how the IDE behaves, autocomplete, etc. maybe catchingResult is the best we can do?

iaik-jheher · 2024-10-16T11:30:01Z

maybe catchingUnwrap or something? it's semantically equivalent to catching { }.unwrap() just without construction overhead

JesusMcCloud · 2024-10-16T11:34:49Z

I went with catchingUnwrapped

iaik-jheher · 2024-10-16T11:38:09Z

kmmresult/src/commonMain/kotlin/at/asitplus/KmmResult.kt

@@ -268,7 +268,7 @@ inline fun <R, T : R> KmmResult<T>.recoverCatching(block: (error: Throwable) ->
 /**
 * Non-fatal-only-catching version of stdlib's [runCatching], directly returning a [KmmResult] --
 * Re-throws any fatal exceptions, such as `OutOfMemoryError`. Relies on [Arrow](https://arrow-kt.io)'s
- * [nonFatalOrThrow](https://apidocs.arrow-kt.io/arrow-core/arrow.core/non-fatal-or-throw.html) internally.
+ * [nonFatalOrThrow](https://apidocs.arrow-kt.io/arrow-core/arrow.core/non-fatal-or-throw.html) logic.


This doesn't make it clear that Arrow is not a dependency imo, and where we took the list from doesn't really matter enough that we'd put it in the docs.

How about A list of fatal exceptions is [here](https://github.com/a-sit-plus/KmmResult/tree/main/bla/blub).?

I want to give credit, where credit is due. how about "Mirrors Arrow's logic"

if you link to our source for nonFatalOrThrow, the comment there references arrow; is that not enough?

I'm gonna go with re-iplements Arrow's logic to avoid a dependency on Arrow for a single function.

sounds good to me

iaik-jheher · 2024-10-16T11:38:49Z

kmmresult/src/commonMain/kotlin/at/asitplus/NonFatal.kt

+expect inline fun Throwable.nonFatalOrThrow(): Throwable
+
+/**
+ * Helper to effectively convert stdlib's [runCatching] to behave like KmmResults Non-fatal-only [catching]. I.e. any


KmmResult's, apostrophe

iaik-jheher · 2024-10-16T11:39:41Z

kmmresult/src/commonMain/kotlin/at/asitplus/NonFatal.kt

+ * Throws any fatal exceptions. This is a re-implementation taken from Arrow's
+ * [`nonFatalOrThrow`](https://apidocs.arrow-kt.io/arrow-core/arrow.core/non-fatal-or-throw.html) –
+ * not because it is bad, it is actually pretty much perfect.
+ * However, the arrow dependency triggered an obscure IDEA bug, resulting in `NoClasDefFoundErrors` instead of correct
+ * behaviour.  * We therefore removed the dependency and added the functionality directly to KmmResult.
+ *
+ * Please note that this was never a problem building anything that depended on KmmResult, it only made debugging
+ * in IDEA a nightmare.


this feels like far too much exposition for an apidoc comment

iaik-jheher · 2024-10-16T11:39:57Z

kmmresult/src/commonMain/kotlin/at/asitplus/NonFatal.kt

+/**
+ * Non-fatal-only-catching version of stdlib's [runCatching], returning a [Result] --
+ * Re-throws any fatal exceptions, such as `OutOfMemoryError`. Relies on [Arrow](https://arrow-kt.io)'s
+ * [nonFatalOrThrow](https://apidocs.arrow-kt.io/arrow-core/arrow.core/non-fatal-or-throw.html) logic.


same point as with catching

iaik-jheher · 2024-10-16T11:40:02Z

kmmresult/src/commonMain/kotlin/at/asitplus/NonFatal.kt

+ * Non-fatal-only-catching version of stdlib's [runCatching] (calling the specified function [block] with `this` value
+ * as its receiver), directly returning a [Result] --
+ * Re-throws any fatal exceptions, such as `OutOfMemoryError`. Relies on [Arrow](https://arrow-kt.io)'s
+ * [nonFatalOrThrow](https://apidocs.arrow-kt.io/arrow-core/arrow.core/non-fatal-or-throw.html) logic.


kmmresult/src/commonTest/kotlin/KmmResultTest.kt

- migrate to dokka 2 for documentation - multi-module project setup - introduce `kmmresult-test`, featuring - `result should succeed` - `result shouldNot succeed` - `result shouldSucceedWith expectedValue` - `result.shouldSucceed()` returning the contained value - remove Arrow dependency and import arrow's list of Fatal exceptions directly into our code - Introduce `Result.nonFatalOrThrow` to mimic KmmResult's non-fatal-only behaviour, but without the object instantiation overhead - Introduce `carchingUnwrapped`, which mimics KmmResult's non-fatal-only behaviour, but without the object instantiation overhead

JesusMcCloud added 2 commits October 9, 2024 09:39

migrate to dokka 2

43932ac

tmp multi-module fail

2652065

JesusMcCloud requested a review from iaik-jheher October 10, 2024 07:00

JesusMcCloud force-pushed the feature/test branch from 2957e3e to 4990844 Compare October 10, 2024 14:39

JesusMcCloud force-pushed the feature/test branch from 4990844 to 87efd21 Compare October 12, 2024 05:27

1.8.0: test extensions, no more arrow deps

5bad8ca

JesusMcCloud force-pushed the feature/test branch from 87efd21 to 5bad8ca Compare October 12, 2024 05:29

nodh requested changes Oct 14, 2024

View reviewed changes

Address PR Feedback

8697729

JesusMcCloud requested a review from nodh October 16, 2024 06:23

nodh approved these changes Oct 16, 2024

View reviewed changes

catchingUnwrapped

2ffd3bd

iaik-jheher reviewed Oct 16, 2024

View reviewed changes

kmmresult/src/commonTest/kotlin/KmmResultTest.kt Show resolved Hide resolved

address more PR feedback

18b1130

JesusMcCloud requested a review from iaik-jheher October 16, 2024 12:18

iaik-jheher approved these changes Oct 16, 2024

View reviewed changes

update changelog

c3a5810

JesusMcCloud merged commit b8111e2 into development Oct 16, 2024
3 checks passed

JesusMcCloud deleted the feature/test branch October 16, 2024 13:27

JesusMcCloud mentioned this pull request Oct 25, 2024

Kotest extensions #7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/test #9

Feature/test #9

JesusMcCloud commented Oct 10, 2024

iaik-jheher commented Oct 11, 2024

JesusMcCloud commented Oct 12, 2024

iaik-jheher commented Oct 14, 2024 •

edited

Loading

iaik-jheher commented Oct 14, 2024

nodh Oct 14, 2024

JesusMcCloud Oct 16, 2024

nodh Oct 14, 2024

JesusMcCloud Oct 16, 2024

nodh Oct 14, 2024

JesusMcCloud Oct 16, 2024

nodh Oct 14, 2024

JesusMcCloud Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher Oct 16, 2024

JesusMcCloud Oct 16, 2024

iaik-jheher Oct 16, 2024

JesusMcCloud Oct 16, 2024

iaik-jheher Oct 16, 2024

iaik-jheher Oct 16, 2024

iaik-jheher Oct 16, 2024

iaik-jheher Oct 16, 2024

iaik-jheher Oct 16, 2024

Feature/test #9

Feature/test #9

Conversation

JesusMcCloud commented Oct 10, 2024

iaik-jheher commented Oct 11, 2024

JesusMcCloud commented Oct 12, 2024

iaik-jheher commented Oct 14, 2024 • edited Loading

iaik-jheher commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

iaik-jheher commented Oct 16, 2024

JesusMcCloud commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iaik-jheher commented Oct 14, 2024 •

edited

Loading