NFIBrokerage / slipstream

A slick WebSocket client for Phoenix Channels
https://hex.pm/packages/slipstream
Apache License 2.0
155 stars 18 forks source link

Allow configuring a serializer and support binary data #56

Closed pojiro closed 1 year ago

pojiro commented 1 year ago

This PR tries to implement handling binary data feature, relates to #28 and #53.

This may include implementation and testing omissions due to a lack of understanding of the Slipstream overview.

Roughly, I first prepare the Serializer behaviour as Slipstream.Serializer, then implement client side silializer which corresponding to Phoenix V2 JSON serializer.

I followed the TDD, first added tests to synchronicity_test.exs and then implemented to satisfy them.

This PR is intended to be the base of implementation and should be modified to fit this project.

pojiro commented 1 year ago

Currentry I refactor lib/slipstream/connection/impl.ex, could you wait a little longer for the review, till it will be finished.

finished.

pojiro commented 1 year ago

@the-mikedavis Could you review this when you are free?

the-mikedavis commented 1 year ago

Thanks for the PR! I am away for a few weeks but I will give the changes a proper look when I get back.

pojiro commented 1 year ago

I fixed the credo issue. I will fix the following coverall issue later, thanks.

https://coveralls.io/builds/60510158/source?filename=lib%2Fslipstream%2Fserializer%2Fphoenix_socket_v2_serializer.ex

pojiro commented 1 year ago

Coverall issues are fixed, thanks.

pojiro commented 1 year ago

Thanks for your review! I added one commit for it. This commit make that

How about this?