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

Add a getGuildOrNull function to the Kord class #714

Merged
merged 6 commits into from
Nov 6, 2022

Conversation

NoComment1105
Copy link
Contributor

Previously there was just a getGuild function that called the nullable variant below the hood, but the non-nullable variant also exists.
I've created the getGuildOrNull function to clearly show it function returns a null, and the getGuild function now calls the non-nullable function, and as such will throw EntityNotFoundException when called and the guild is null.

This could be quite a breaking change for bots, as the getGuild function no longer returns null, it throws. This should be announced somewhere, or a release created to document.

I also KDoc'd these functions :)

@lukellmann
Copy link
Member

That's why the build fails:

e: warnings found and -Werror specified
w: /home/runner/work/kord/kord/core/src/test/kotlin/rest/RestTest.kt: (89, 60): Unnecessary non-null assertion (!!) on a non-null receiver of type Guild
w: /home/runner/work/kord/kord/core/src/test/kotlin/rest/RestTest.kt: (558, 49): Unnecessary non-null assertion (!!) on a non-null receiver of type Guild
w: /home/runner/work/kord/kord/core/src/test/kotlin/rest/RestTest.kt: (570, 49): Unnecessary non-null assertion (!!) on a non-null receiver of type Guild

@NoComment1105
Copy link
Contributor Author

Aaa, thank, will fix in a bit

@lukellmann
Copy link
Member

This could be quite a breaking change for bots, as the getGuild function no longer returns null, it throws. This should be announced somewhere, or a release created to document.

The alternative would be to deprecate getGuild that can return null, replace it with getGuildOrNull and then later reintroduce getGuild that throws - this wouldn't be a breaking change but take longer until it's fully released.

@NoComment1105
Copy link
Contributor Author

That seems reasonable, personally I don't like the idea of going without the throwing function, maybe using JVM name annotations to allow both to exist could be used?

@lukellmann
Copy link
Member

Problem with that is that it might be binary compatible but it would still silently remove null returns.

We could maybe add a temporaty function like getGuildOrThrow that will then be deprecated and removed again when getGuild is converted to throwing behavior.

@NoComment1105
Copy link
Contributor Author

Yeah that seems reasonable to me,if you're ok with that

Copy link
Member

@lukellmann lukellmann left a comment

Choose a reason for hiding this comment

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

Thanks! :)

@lukellmann lukellmann merged commit 4e74545 into kordlib:0.8.x Nov 6, 2022
@NoComment1105 NoComment1105 deleted the getGuildOrNull branch November 7, 2022 17:23
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