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

App ws auth and remove AppWebsocket #64

Merged
merged 7 commits into from
Apr 24, 2024
Merged

App ws auth and remove AppWebsocket #64

merged 7 commits into from
Apr 24, 2024

Conversation

ThetaSinner
Copy link
Member

@ThetaSinner ThetaSinner commented Apr 24, 2024

Note that the AppWebsocket isn't completely gone because it is still necessary to construct in two steps to be able to get AppInfo as part of construction. However, it is gone from the crate interface!

@ThetaSinner ThetaSinner requested a review from jost-s April 24, 2024 16:39
Copy link
Contributor

@jost-s jost-s left a comment

Choose a reason for hiding this comment

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

LGTM
I suggest we rename AppAgentWebsocket to AppWebsocket and use a different identifier for the now internal app websocket. What do you think?

src/admin_websocket.rs Outdated Show resolved Hide resolved
@ThetaSinner
Copy link
Member Author

Great idea, rename done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Rename this to app_websocket.rs?

@ThetaSinner ThetaSinner merged commit 3d6ae11 into develop Apr 24, 2024
2 checks passed
@ThetaSinner ThetaSinner deleted the app-ws-auth branch April 24, 2024 21:42
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.

2 participants