Skip to content

Conversation

yakivy
Copy link
Contributor

@yakivy yakivy commented Jul 11, 2025

@Mr3zee Mr3zee self-requested a review July 14, 2025 17:29
Copy link
Member

@Mr3zee Mr3zee left a comment

Choose a reason for hiding this comment

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

@yakivy Hey! Thanks for the contribution! I'll accept that, but can you please move the test into krpc/krpc-test/src/commonMain/kotlin/kotlinx/rpc/krpc/test/KrpcTransportTestBase.kt? Sampling is for the protocol testing purposes

You can see how LocalDateSerializer is used there for contextual serialization. Tests don't run in IDE, so you can check it with ./gradlew krpc:krpc-test:jvmTest --tests "*<test_name>*"

@Mr3zee Mr3zee added the bug Something isn't working label Jul 14, 2025
@yakivy yakivy force-pushed the regression-nullable-contextual-serializers branch from 4c61fc3 to 5e1beea Compare July 14, 2025 18:27
@yakivy
Copy link
Contributor Author

yakivy commented Jul 14, 2025

@Mr3zee lol, I knew there should be an easier way! Thanks for pointing out. Could you take a look?

@Mr3zee
Copy link
Member

Mr3zee commented Jul 14, 2025

@yakivy yep, thanks! One last thing: please put the test in the base class and just ignore it in protobuf. You can see how it's done for other nullable test

@yakivy yakivy force-pushed the regression-nullable-contextual-serializers branch from 5e1beea to a9f38f2 Compare July 14, 2025 19:25
@yakivy
Copy link
Contributor Author

yakivy commented Jul 14, 2025

@Mr3zee done

@Mr3zee Mr3zee merged commit 755a290 into Kotlin:main Jul 15, 2025
13 checks passed
@Mr3zee
Copy link
Member

Mr3zee commented Jul 15, 2025

@yakivy thanks for the contribution!

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants