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

REPLACE Moshi by Kotlinx serialization #42

Merged
merged 8 commits into from
Feb 29, 2024
Merged

Conversation

ninovanhooff
Copy link
Collaborator

@ninovanhooff ninovanhooff commented Dec 15, 2023

Why is this important?

Moshi allows for more programmer error when forgetting to add annotations. Also it needs an extra plugin for sealed classes support and it needs proguard rules

We use Kotlinx.serialization for all json parsing
because it is fast, modern and has IDE integration (which warns you when you forget to add @serializable annotations).
It also has multiplatform support, so we can use it in our KMP projects as well.

Any review notes?

This false positive linting warning. The project runs fine.
For a template project, these warnings might not be acceptable

It seems scheduled to be fixed in intellij 2023.3, which is Android Studio Jellyfish, currently in Nightly (did not test myself)

Strangely, I do not have this issue in the widm project (which uses other versions of kotlin and the gradle plugin, same Android Studio)
https://github.com/Q42/widm-android/blob/689d6e71935f460b2791c73fc505f681b40dcafb/gradle/libs.versions.toml

https://youtrack.jetbrains.com/issue/KTIJ-10755/HMPP-IDE-False-PLUGINISNOTENABLED-in-IDE-for-kotlinx-serialization-plugin-however-project-builds-correctly

image

@ninovanhooff ninovanhooff changed the title Kotlinx serialization REPLACE Moshi by Kotlinx serialization Dec 15, 2023
@ninovanhooff ninovanhooff marked this pull request as ready for review December 15, 2023 15:24
This allows reverse proxies like Charles to inspect and manipulate traffic in development builds
sebaslogen
sebaslogen previously approved these changes Dec 21, 2023
Copy link
Collaborator

@sebaslogen sebaslogen left a comment

Choose a reason for hiding this comment

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

Looking and working fine. Just a couple of small suggestions, up to you.
The warning is fine, it's not related to our code but a tooling issue that will eventually get fixed.

gradle/libs.versions.toml Outdated Show resolved Hide resolved
build.dep.json.gradle Outdated Show resolved Hide resolved
core/actionresult/build.gradle Outdated Show resolved Hide resolved
build.gradle Show resolved Hide resolved
Copy link
Collaborator

@Frank1234 Frank1234 left a comment

Choose a reason for hiding this comment

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

well done!

@ninovanhooff ninovanhooff merged commit 8ba3443 into main Feb 29, 2024
1 check passed
@ninovanhooff ninovanhooff deleted the kotlinx-serialization branch February 29, 2024 12:58
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.

3 participants