-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds a andThen extension method to Validated to chain Validated #2539
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! Thank you @magott!! 🙏 👏 🥳
I left a nit, and a tip but this can also be merged as is 👍
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Validated.kt
Outdated
Show resolved
Hide resolved
arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/ValidatedTest.kt
Outdated
Show resolved
Hide resolved
72ac91b
to
5b9f78f
Compare
Updated the PR with the suggestions implemented |
Thanks for the contribution @magott !! 🙌 |
Should become available in 1.0.1 with Kotlin 1.5.31 soon ;) |
* | ||
* suspend fun main() { | ||
* val houseNumber = config.parse(Read.intRead, "house_number").andThen { number -> | ||
* if (number >= 0) Valid(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@magott should it rather be Valid(number)
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕵️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, surprised there weren't more errors in code written in doc. Do you want a new PR @nomisRev ? Or will you just fix it in main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Good job 👏 Looking forward to delete the extension method from our code ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix it directly in main.
This is actually why I'm working on this Dokka Plugin: https://github.com/nomisRev/AnkDokkaPlugin/pull/7/files
I think we can already do it today too with our current setup, but if we wrote tests instead of examples than we could catch these mistakes on CI as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think the docs compile as part of the build, so this isn't an error as such. Just a logical bug.
As discussed on slack [https://kotlinlang.slack.com/archives/C5UPMM0A0/p1632399994123800] proposing a andThen method for
Validated
to easier chain Validations without going throughwithEither