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

Rust examples: Improve rooms registry logic #1146

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

Jurshsmith
Copy link
Contributor

This change removes the redundant logic when
getting or creating a room.

@nazar-pc
Copy link
Collaborator

Thanks, I'll have to look at it carefully later

@Jurshsmith
Copy link
Contributor Author

Thanks @nazar-pc

I am looking into the linting issue.

So that you know, this is just 1 of many PRs I will be creating. My main goal is to decouple the ParticipantConnection from the WebSocket connection to serve as a more generic integration template when working with non-Actix frameworks like Warp, Axum, or bare Tokio

@nazar-pc
Copy link
Collaborator

These examples are not designed to be end-user applications even remotely, there is quite a lot more involved in production application. They are just a trivial showcase how to use the library and how to have a few more things around it. Supporting more frameworks is really not the goal here and decoupling will likely require more abstractions that will make code harder to understand, not easier.

@Jurshsmith
Copy link
Contributor Author

Jurshsmith commented Aug 29, 2023

These examples are not designed to be end-user applications even remotely, there is quite a lot more involved in production applications.

Agreed. I plan to keep existing features as is.

...and decoupling will likely require more abstractions that will make code harder to understand, not easier.

Not really. Personally, it was initially difficult to understand until I saw the boundary between the participant connection and the WebSocket connection. I think explicitly showing that boundary will simplify it a bit. I don't foresee a lot of abstractions besides a WebsocketMessageHandler trait. What do you think?

By the way, it seems the CI failure is environment-related (Something with the Rust version).

@nazar-pc
Copy link
Collaborator

v3 branch doesn't have Cargo.lock committed (WIP flatbuffers branch does), feel free to upgrade rust-toolchain.toml to latest stable release and fix all of the clippy lints afterwards if there are any. Ideally as a separate PR.

This change removes the redundant logic when
getting or creating a room.
@Jurshsmith Jurshsmith force-pushed the improve-rooms-registry-logic branch from d27e4e2 to dc7bb7e Compare August 30, 2023 21:35
@ibc
Copy link
Member

ibc commented Oct 23, 2023

As per above discussion, can we close this PR?

@nazar-pc
Copy link
Collaborator

I still need to look into this, but as postponing, will try to review this week

@jmillan jmillan changed the title Improve rooms registry logic Rust examples: Improve rooms registry logic Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants