Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

Initial WebSocket implementation. #25

Merged
merged 2 commits into from
Mar 28, 2021
Merged

Initial WebSocket implementation. #25

merged 2 commits into from
Mar 28, 2021

Conversation

mucinoab
Copy link
Collaborator

@mucinoab mucinoab commented Mar 12, 2021

TODO

  • async
  • proper error handling
  • improve documentation
  • better names
  • more tests

@nph4rd
Copy link
Owner

nph4rd commented Mar 21, 2021

@mucinoab - this is looking great! Thanks so much for this! 🎆

I've had the chance to look through most of the code included so far and I think most of my observations are covered by the checklist in the description. I noticed there are lots of unwraps atm. I should think "proper error handling" will allow us to get rid of them. Apart from that I would add the following:

  • I would suggest to move some of the structs in websocket.rs into model/. This would be consistent with the rest of the code in terms of modularity. Also, the deserialize_books function could be in util.rs.

  • Noticed there is an example in the docs for the BitsoWebSocket struct. I think it might be worth adding something like this to examples/ - again, in order to be consistent with the rest of the project.

All in all, I think it's looking brilliant! Let me know if there is something I can do to keep moving this forward 😄 👍

@mucinoab mucinoab marked this pull request as ready for review March 27, 2021 22:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants