Skip to content
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

Mention ArrowModule in serialization and ktor sections #306

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tKe
Copy link

@tKe tKe commented May 14, 2024

Given the ArrowModule is currently only in the 2.0 branch, I would assume this will need to wait until the 2.0 release. As such, I've also removed the reference to ValidatedSerializer.

Copy link
Member

@serras serras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including the new information. I'm just requesting a few changes to ensure that the build succeeds.

@@ -78,6 +78,16 @@ The main point of contact between Ktor and Arrow is in
If you're using kotlinx.serialization, you need no further changes other than
importing the serializers with `@UseSerializers`.

If you want to use Arrow Core types directly as your request or response models, you will need to include the `ArrowModule` in your serializers module:

```kotlin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, do not use ```kotlin here (or in the rest of the file), as it would trigger Knit, but these blocks are not complete Kotlin code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the kotlin tags from the code blocks, and also from the pre-existing blocks for consistency.

They weren't being picked up by knit in the build (at least a ./gradlew knit build passed) as there were no knit patterns in either file, but should one be added at a later it's better to avoid surprises.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants