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.

Sign up for GitHub

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

Jump to bottom

Fix serenity breaking changes #173

Merged
merged 7 commits into from
Apr 12, 2023
Merged

Fix serenity breaking changes #173

merged 7 commits into from
Apr 12, 2023

Conversation

tazz4843
Copy link
Contributor

@tazz4843 tazz4843 commented Apr 9, 2023

Caused by serenity-rs/serenity#2372.

Currently untested, will test when I get a chance to do so.

@kangalio
Copy link

kangalio commented Apr 9, 2023

As the one who made the breaking PR - can you give feedback, do the changes seem logical? I see you had to change serenity::gateway::InterMessage to serenity::client::bridge::gateway::ShardRunnerMessage. The upcoming serenity-rs/serenity#2380 will change the ugly long path to be serenity::gateway::ShardRunnerMessage again.

@tazz4843
Copy link
Contributor Author

tazz4843 commented Apr 9, 2023

I was very confused as to what InterMessage got replaced with. The changelog entry you provided in the linked PR didn't really help to provide a replacement, so I had to look through the commits to discover.

@kangalio
Copy link

kangalio commented Apr 9, 2023

I was very confused as to what InterMessage got replaced with. The changelog entry you provided in the linked PR didn't really help to provide a replacement, so I had to look through the commits to discover.

Thanks, you're right. I amended some additional context in serenity-rs/serenity#2372 (comment)

@tazz4843
Copy link
Contributor Author

tazz4843 commented Apr 9, 2023

Since serenity-rs/serenity#2380 is coming soon, this PR should probably wait for it to be merged.

@FelixMcFelix
Copy link
Member

FelixMcFelix commented Apr 11, 2023 •
edited
Loading

@tazz4843 I've re-run CI, serenity-rs/serenity#2380 has indeed broken the fix.

@kangalio
Copy link

To update, just replace serenity::client::bridge::gateway with serenity::gateway in the code

@FelixMcFelix
Update references to serenity::gateway
a72da28 
Additionally, Box the TrySendError from Serenity at Clippy's behest (~330B error type).
@FelixMcFelix FelixMcFelix merged commit 275e207 into serenity-rs:next Apr 12, 2023
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request Nov 20, 2023
@tazz4843 @FelixMcFelix
Gateway: Fix serenity breaking changes (serenity-rs#173)
4d0c1c0 
Fix issues caused by serenity-rs/serenity#2372 and serenity-rs/serenity#2380.

Additionally, this Boxes the TrySendError from Serenity at Clippy's behest (causing a ~330B Result type).

---------

Co-authored-by: Kyle Simpson <kyleandrew.simpson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tazz4843 @kangalio @FelixMcFelix