warp
warp copied to clipboard
Make the websocket Message easier to work with
Currently the Message
type is a bit hard to work with as can be seen here: https://github.com/communityvi/communityvi/blob/26bfbe16c8c4dc46545edeebf96d2b3aed8887ab/communityvi-server/src/utils/websocket_message_conversion.rs#L4-L33
While looking at the code I also happened to notice the comment about non-exhaustive
enum:
https://github.com/seanmonstar/warp/blob/9e74ff98e0f314879c29cf761a372840606bd585/src/filters/ws.rs#L267-L274
It would of course be best for usability/interoperability if warp used tungstenite::protocol::Message
directly. I assume the opaque Message
type was introduced to allow for changes in the underlying implementation, without breaking warp's API?
This pull request contains 2 commits:
- Convert
Message
into anon-exhaustive
enum without breaking the existing API - Add
From
fortungstenite::protocol::Message
in both directions. This doesn't break the API, but it exposes the respective version of thetungstenite::protocol::Message
, making any future update of thetungstenite
dependency a potentially breaking change. If that is undesired, I can remove this second commit.
Rebased to fix formatting
Hi, and thanks for your interest! There is already https://github.com/seanmonstar/warp/pull/821 on the pipeline
Sorry for not doing due diligence and checking for existing pull requests.
How should I proceed? Close this in favor of the existing pull request? I've noticed that there hasn't been any progress in the original pull request for several months.
no worries! :) yeah I was just noticing that as well, and this one already addresses my review on the other and uses From<protocol::Message>
which is cleaner LGTM!
Sorry for bumping, but is there anything I could do to help get this merged?