-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
codec: simplify json codec. #3524
Conversation
LGTM |
util/types/json/serdes.go
Outdated
return | ||
} | ||
} | ||
err = errors.New("Invalid JSON bytes") |
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.
Is it better to return different error message for 1. len(b) == 0
, 2. b[0]
is invalid ?
util/types/json/json_test.go
Outdated
@@ -54,15 +54,16 @@ func (s *testJSONSuite) TestSerializeAndDeserialize(c *C) { | |||
j2 := mustParseFromString(`[{"a": 1, "b": true}, 3, 3.5, "hello, world", null, true]`) | |||
|
|||
var testcses = []struct { |
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.
s/testcses/testcases
util/types/json/serdes.go
Outdated
// PeekBytesAsJSON trys to peek some bytes from b, until | ||
// we can deserialize a JSON from those bytes. | ||
func PeekBytesAsJSON(b []byte) (n int, err error) { | ||
if len(b) > 0 { |
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.
use len(b) <= 0 better?
thus avoiding the indentation
@@ -93,6 +93,43 @@ import ( | |||
// lengths up to 16383, and so on... | |||
*/ | |||
|
|||
const ( |
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.
add comments for these constants.
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.
The constant 4 is used somewhere else in this file, we could use a const variable for all of them.
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.
fixed.
util/types/json/serdes.go
Outdated
// we can deserialize a JSON from those bytes. | ||
func PeekBytesAsJSON(b []byte) (n int, err error) { | ||
if len(b) > 0 { | ||
switch c := b[0]; c { |
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.
Is TypeCode(b[0]) available?
If so, may be more readable.
Rest LGTM |
LGTM |
util/types/json/serdes.go
Outdated
@@ -97,34 +97,39 @@ const ( | |||
typeCodeLen int = 1 |
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.
comment for these constant?
No description provided.