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

And nullable/nonnullable annotation to improve Kotlin API #54

Merged
merged 6 commits into from
Feb 13, 2023

Conversation

PhilipDukhov
Copy link
Contributor

Right now, when used from Kotlin, library generates a lot of optional parameters - that's default behaviour for Java interop, as there's no build-in nullability checks.

object : SubscriptionEventListener() {
    override fun onSubscribed(sub: Subscription?, event: SubscribedEvent?) {
    }

With this PR I've added ParametersAreNonnullByDefault annotation to the lib package - this makes all arguments non nullable by default for IDEA linter and for Kotlin interop.

Also I've added a couple Nullable annotation for parameters that actually can be null - after adding @ParametersAreNonnullByDefault, IDEA showed me such places with warnings.

Also fixed a couple of warned placed

@FZambia
Copy link
Member

FZambia commented Feb 13, 2023

@PhilipDukhov many thanks, I'll go through the changes one more time very soon and if no other comments will appear proceed with merging.

@FZambia FZambia merged commit 1e69b4d into centrifugal:master Feb 13, 2023
@FZambia
Copy link
Member

FZambia commented Feb 13, 2023

Awesome 👍

@PhilipDukhov PhilipDukhov deleted the nonnullable branch February 14, 2023 07:27
@FZambia
Copy link
Member

FZambia commented Feb 14, 2023

@PhilipDukhov hello, unfortunately I think this requires more careful research. I just noticed that all my Java code that uses this SDK started to have various warnings like this:

Screenshot 2023-02-14 at 23 44 57

Also it started complaining to possible NullPointerException where it can't be thrown:

Screenshot 2023-02-14 at 23 54 59

I see more clearly now how Result type may help here 😬

Not sure this all can be fixed quickly. Seems that I'll have to revert the change for now :(

@FZambia
Copy link
Member

FZambia commented Feb 15, 2023

Opened #61

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

Successfully merging this pull request may close these issues.

2 participants