-
Notifications
You must be signed in to change notification settings - Fork 31
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
Use new msgpack package #65
Conversation
These functions are used to encode and decode msgpack using github.com/ugorji/go/codec library
Use []byte instead of string, codec seems to support []byte and defaults to it for strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments only. Looks good!
ably/realtime_client_test.go
Outdated
@@ -90,7 +90,7 @@ func checkUnique(ch chan string, typ string, n int) error { | |||
} | |||
|
|||
func TestRealtimeClient_50clients(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well rename the test
"bytes" | ||
"io" | ||
|
||
"github.com/ugorji/go/codec" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be vendored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not in this PR. I plan to make Godep directory go away by the time we integrate the integration branch to develop and use the new go module
to manage dependencies.
But that work will be done later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, makes sense
As a note, this PR was necessary because the original msgpack decoder would only decode double values to a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, looks good. Thank you
This PR introduces github.com/ugorji/go/codec as mspack encoder/decoder