Skip to content
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

feat: change json lib to jsoniter #990

Merged
merged 4 commits into from
Jul 8, 2017
Merged

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented Jul 8, 2017

A high-performance 100% compatible drop-in replacement of "encoding/json"

https://github.com/json-iterator/go

cc @javierprovecho @tboerger

Signed-off-by: Bo-Yi Wu appleboy.tw@gmail.com

A high-performance 100% compatible drop-in replacement of "encoding/json"

https://github.com/json-iterator/go

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@javierprovecho
Copy link
Member

javierprovecho commented Jul 8, 2017

@appleboy test failed:

=== RUN   TestContextRenderIndentedJSON
--- FAIL: TestContextRenderIndentedJSON (0.00s)
	Error Trace:	context_test.go:585
	Error:		Not equal: 
			expected: "{\n    \"foo\":\"bar\",\n    \"bar\":\"foo\",\n    \"nested\":{\n        \"foo\":\"bar\"\n    }\n}"
			received: "{\n    \"bar\": \"foo\",\n    \"foo\": \"bar\",\n    \"nested\": {\n        \"foo\": \"bar\"\n    }\n}"
=== RUN   TestErrorSlice
--- FAIL: TestErrorSlice (0.00s)
	Error Trace:	errors_test.go:94
	Error:		Not equal: 
			expected: "[{\"error\":\"first\"},{\"meta\":\"some data\",\"error\":\"second\"},{\"status\":\"400\",\"error\":\"third\"}]"
			received: "[{\"error\":\"first\"},{\"error\":\"second\",\"meta\":\"some data\"},{\"error\":\"third\",\"status\":\"400\"}]"
	

Both of them fail because json-ite sort the order of keys.

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@appleboy
Copy link
Member Author

appleboy commented Jul 8, 2017

@javierprovecho I already fixed it this commit 08338ef

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@javierprovecho
Copy link
Member

@appleboy it will keep failing, the tests here are trivial, json-ite outputs the json without ordering

@codecov
Copy link

codecov bot commented Jul 8, 2017

Codecov Report

Merging #990 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #990   +/-   ##
=======================================
  Coverage   96.52%   96.52%           
=======================================
  Files          16       16           
  Lines        1412     1412           
=======================================
  Hits         1363     1363           
  Misses         39       39           
  Partials       10       10
Impacted Files Coverage Δ
errors.go 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4249f9...e23842e. Read the comment docs.

@appleboy
Copy link
Member Author

appleboy commented Jul 8, 2017

@javierprovecho Yes. maybe we need open the issue to https://github.com/json-iterator/go

@javierprovecho
Copy link
Member

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@javierprovecho javierprovecho reopened this Jul 8, 2017
@javierprovecho javierprovecho merged commit ad08765 into gin-gonic:master Jul 8, 2017
@appleboy appleboy deleted the json branch July 8, 2017 10:52
@codedance
Copy link

jsoniter also does not seem to work on Google App Engine (use of unsafe). One option may be to enable/disable this via build tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants