-
-
Notifications
You must be signed in to change notification settings - Fork 5
Replace jda-ktx with our own module #239
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed ICoroutineEventManagerSupplier - Added CoroutineEventListener
Then simplify dependencies
Use if-null checks before using setters to benefit from the compiler eliding the calls Constant evaluation doesn't work on properties (yet), but end users should still benefit from it if later versions do have it
Users can still somewhat choose which one they want to use
…ith both core and jda-ktx import replacements
5797706 to
9856d5d
Compare
Will be in release notes instead
…eveThreadChannelOrNull`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes jda-ktx and introduces a new
BotCommands-jda-ktxmodule, this is done for a few reasons:This will no longer include jda-ktx transitively, you are recommended to migrate, but if you choose to add it back, be aware that functions relying on
CoroutineEventManager, such asJDA#await/listenwill no longer work.Please look at the README for more details.
Differences with
jda-ktxChanges to the
CoroutineEventManagerThe core module includes its own event manager, you can configure its default timeout at
BEventManagerConfig#defaultTimeout, and theCoroutineScopeinBCoroutineScopesConfig#eventManagerScope.As a result, extensions from
jda-ktxrelying on itsCoroutineEventManagerno longer work, but you can fix this by importing the extension from this module.You can still register
CoroutineEventListeners (the one fromBotCommands-core, notjda-ktx!) if required.Changed functionalities
If you decide to switch, some functions have replacements:
awaitMessagemust now be used on anEventWaiterinstanceawaitButtonis nowButton#await(the framework'sButtonclass)jda-ktx'sPaginatoris replaced by the corePaginatorsIncompatible functionalities
If you keep using
jda-ktx, some functions will not work or behave unexpectedly:scopeonJDA/ShardManagerJDAService.Missing functionalities
Additionally,
jda-ktxhas additional extensions that were not ported:String#toEmoji()String#toCustomEmoji()String#toUnicodeEmoji()UnicodeEmojisclass (included by default)getDefaultScopenamedDefaultScopeWebhookAppendernamed,String#invokeand someintoextensions from messages/utils.ktCallextensionsawaitWithandawaitprivate val logger by SLF4J)private val logger = KotlinLogging.logger { })Loggingclass may also be of interestrefextensions on entitiesPlease open an issue if you feel like one of these extensions are worth porting, an alternative is to port them locally.
Migrating
You can apply an OpenRewrite recipe to change all relevant imports, note that this could still require you to do some changes after it.
Before migrating, your codebase must compile, so you must not update any versions yet.
Make sure you have committed your current changes, or make a back up of your project.
Registering the recipe
Note
This will log warnings for Kotlin sources, but the recipes still work fine.
(Kotlin) Gradle
plugins { // ... id("org.openrewrite.rewrite") version "7.11.0" } repositories { // ... maven("https://jitpack.io") } dependencies { // Existing dependencies // ... rewrite("io.github.freya022:BotCommands-jda-ktx:{{latest commit hash}}") rewrite("org.openrewrite.recipe:rewrite-java-dependencies:1.37.0") } rewrite { // Use 'dev.freya02.MigrateToBcJdaKtx' if you want to migrate from jda-ktx (recommended) // Use 'dev.freya02.MigrateFromBcCoreToBcJdaKtx' if you want to keep using jda-ktx activeRecipe("dev.freya02.MigrateToBcJdaKtx") }Checking changes (dry run)
Run the
rewriteDryRuntask, this can be done by pressing CTRL twice on IntelliJ then runninggradle rewriteDryRun.This will generate a diff file at
build/reports/rewrite/rewrite.patch, you can check the possible changes there.Applying changes
Run the
rewriteRuntask.Maven
Checking changes (dry run)
Run the
rewrite:dryRungoal, this can be done by pressing CTRL twice on IntelliJ then runningmvn rewrite:dryRun.This will generate a diff file at
target/site/rewrite/rewrite.patch, you can check the possible changes there.Applying changes
Run the
rewrite:rungoal.You can then update
BotCommands-coreand addBotCommands-jda-ktx.It is recommended to optimize imports (
Code | Optimize importsin IntelliJ) after migrating.Notes
Breaking changes
ICoroutineEventManagerSupplierBEventManagerConfig#defaultTimeoutBCoroutineScopesConfig#eventManagerScopejda-ktxapi dependencyBotCommands-jda-ktxNew Features
CoroutineEventListenerBotCommands-jda-ktxChanges
BotCommands-jda-ktxRestAction,RestResultand related, which have vararg parameters, now require at least 1 argument