- Notifications
You must be signed in to change notification settings - Fork 111
update to latest jda version #130
Conversation
| JDA-Utilities has been dead since quite a while, so I doubt this will be merged anytime soon. You might want to look into forks or even a different library. |
| You broke several core functionalities of "Builder" classes by returning void disallowing chaining. This PR will most likely not be merged unless you undo most changes breaking how JDA Utils is set up. |
| As it was mentioned is this project abandoned. Also, as sanduhr mentioned does your PR also break everything. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| CooldownScope.USER.genKey(name, event.getAuthor().getIdLong()); | ||
| case SHARD: | ||
| event.getJDA().getShardInfo(); | ||
| return cooldownScope.genKey(name, event.getJDA().getShardInfo().getShardId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes no sense and breaks logic
| return cooldownScope.genKey(name, event.getJDA().getShardInfo().getShardId()); | ||
| case USER_SHARD: | ||
| event.getJDA().getShardInfo(); | ||
| return cooldownScope.genKey(name,event.getAuthor().getIdLong(),event.getJDA().getShardInfo().getShardId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes no sense and breaks logic
| * @return This CommandBuilder | ||
| */ | ||
| public CommandBuilder setName(String name) | ||
| public void setName(String name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change and makes the builder less useful as you remove the fluent interface pattern for no reason.
| { | ||
| return event.getGuild() == null ? null : event.getGuild().getSelfMember(); | ||
| event.getGuild(); | ||
| return event.getGuild().getSelfMember(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again this change makes no sense and breaks logic
| | ||
| jda.getShardInfo(); | ||
| bodyBuilder.add("shard_id", Integer.toString(jda.getShardInfo().getShardId())) | ||
| .add("shard_count", Integer.toString(jda.getShardInfo().getShardTotal())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, nonsense change.
| { | ||
| LOG.info("Successfully sent information to discord.bots.gg"); | ||
| try(Reader reader = response.body().charStream()) | ||
| try(Reader reader = Objects.requireNonNull(response.body()).charStream()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requreNonNull is useless here
| public static final String VERSION_MINOR = "@VERSION_MINOR@"; | ||
| public static final String VERSION_REVISION = "@VERSION_REVISION@"; | ||
| public static final String VERSION = VERSION_MAJOR.startsWith("@")? "DEV" : VERSION_MAJOR + "." + VERSION_MINOR + "." + VERSION_REVISION; | ||
| public static final String VERSION = "DEV"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change breaks versions entirely
| // currently being displayed. | ||
| if(!event.getMessageId().equals(m.getId())) | ||
| return false; | ||
| r.queue(v -> waiter.waitForEvent(MessageReactionAddEvent.class, event -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes the code harder to read
| return false; | ||
| | ||
| return guild.getMember(user).getRoles().stream().anyMatch(roles::contains); | ||
| return Objects.requireNonNull(guild.getMember(user)).getRoles().stream().anyMatch(roles::contains); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless
| } | ||
| catch(Throwable t) | ||
| { | ||
| } catch (Throwable t) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are changing code style for no reason
| Closing since the majority of changes introduced by this PR are completely unrelated to updating JDA. They are mostly superfluous and in some cases outright breaking and even introduce bugs. |
Pull Request
Pull Request Checklist
Please follow the following steps before opening this PR.
PRs that do not complete the checklist will be subject to denial for
missing information.
or merged features/bug fixes.
Pull Request Information
Check and fill in the blanks for all that apply:
______module of the JDA-Utilities library.______.Description
I updated jda version to latest and small fix codes