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

Introduce Call::current_channel #60

Merged
merged 2 commits into from
Apr 10, 2021

Conversation

DoumanAsh
Copy link
Contributor

I'm not sure how you want to organize connection info and whether channel id should be separate from it.

For now I left it separated, just getting API to access it for user so that I could know whether I need to join new one or not

Fixes #58

Copy link
Member

@FelixMcFelix FelixMcFelix left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This is useful, and can be modified easily to account for #47's ConnectionInfo changes when it's time to release that.

Just need an additional comment on a known bug until next breaking due to #47 (since this PR will go into a patch release after #56 is fixed). Apart from that, lgtm!

@@ -240,6 +240,15 @@ impl Call {
}
}

/// Returns `id` of the channel, if connected to any.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Returns `id` of the channel, if connected to any.
/// Returns `id` of the channel, if connected to any.
///
/// **Note**: The returned `id` will not correctly update if a connected bot
/// is moved between channels manually. This will be fixed in the next
// breaking release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expanded a bit your suggestion. Check it out

Copy link
Member

@FelixMcFelix FelixMcFelix left a comment

Choose a reason for hiding this comment

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

I think that expands the explanation nicely, thanks!

@FelixMcFelix FelixMcFelix merged commit 22214a0 into serenity-rs:current Apr 10, 2021
@DoumanAsh DoumanAsh deleted the call_current_channel branch April 10, 2021 12:22
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request Apr 11, 2021
…ng changes.

This commit undoes serenity-rs#64 (and bumps the library MSRV accordingly), and modifies serenity-rs#60 to match the new `Call` connection handling.

This was tested using `cargo make ready`, and rustc v1.49.0 on `examples/serenity/voice`.
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request May 6, 2021
…ng changes.

This commit undoes serenity-rs#64 (and bumps the library MSRV accordingly), and modifies serenity-rs#60 to match the new `Call` connection handling.

This was tested using `cargo make ready`, and rustc v1.49.0 on `examples/serenity/voice`.
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request May 10, 2021
…ng changes.

This commit undoes serenity-rs#64 (and bumps the library MSRV accordingly), and modifies serenity-rs#60 to match the new `Call` connection handling.

This was tested using `cargo make ready`, and rustc v1.49.0 on `examples/serenity/voice`.
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request Jun 14, 2021
…ng changes.

This commit undoes serenity-rs#64 (and bumps the library MSRV accordingly), and modifies serenity-rs#60 to match the new `Call` connection handling.

This was tested using `cargo make ready`, and rustc v1.49.0 on `examples/serenity/voice`.
FelixMcFelix added a commit to FelixMcFelix/songbird that referenced this pull request Jul 1, 2021
…ng changes.

This commit undoes serenity-rs#64 (and bumps the library MSRV accordingly), and modifies serenity-rs#60 to match the new `Call` connection handling.

This was tested using `cargo make ready`, and rustc v1.49.0 on `examples/serenity/voice`.
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.

Call::connection should containt channel id to determine if you're already in channel
2 participants