-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Add support for Protobuf format response and unit test #1479
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1479 +/- ##
==========================================
+ Coverage 99.15% 99.16% +<.01%
==========================================
Files 36 37 +1
Lines 1894 1905 +11
==========================================
+ Hits 1878 1889 +11
Misses 13 13
Partials 3 3
Continue to review full report at Codecov.
|
Oh my god, the codecov is too strict 😂 |
@salamer please add some test case, thanks a lot! |
context_test.go
Outdated
@@ -17,11 +17,12 @@ import ( | |||
"testing" | |||
"time" | |||
|
|||
"io" |
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.
order by first character.
I'm sorry, I'm too busy these days. |
all code coverage > 99%
mkdir a test data dir.
"not match neither" means that it will match.
* add issue/pr template explain * add issue/pr template explain
…-gonic#1478) * docs: add changelog for v1.3.0, update authors and version const * add link for every referenced pull request (gin-gonic#1481) * docs: add changelog for v1.3.0, update authors and version const * add link for pr
upgrade lib version, and upgrade `github.com/json-iterator/go` to add two libs.
Add BindXML AND ShouldBindXML gin-gonic#1484
eafd9bf
to
b6cb77e
Compare
d5902a6
to
86fe335
Compare
Oh my god, the 99.15% coverage is too strict. I have already filled all my new function test. could it be a little bit lower? |
context_test.go
Outdated
protoData, err := proto.Marshal(data) | ||
assert.NoError(t, err) | ||
|
||
assert.Equal(t, 201, w.Code) |
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.
Please change 201
to http status code in net/http
package.
render/protobuf.go
Outdated
// Copyright 2014 Manu Martinez-Almeida. All rights reserved. | ||
// Use of this source code is governed by a MIT style | ||
// license that can be found in the LICENSE file. | ||
|
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.
Please change to
// Copyright 2018 Gin Core Team. All rights reserved.
// Use of this source code is governed by a MIT style
// license that can be found in the LICENSE file.
fixed missed code bug replace 201 with http lib and update annotation
da3709c
to
75aa0c0
Compare
@appleboy Hi, I have already go pass all the test and update my code as what you commented. |
@salamer Yes. It would be a feature in v1.4 now. Please add an example in README. |
…date some annotations
9488723
to
e51d726
Compare
@appleboy @thinkerou Hi, I'm sorry I'm too busy these days, I can only submit code intermittently 😔. and I have already finished this feature and add clear example and annotation. I hope it can be merged😂😂 |
@thinkerou need your approval. |
@appleboy done |
Gin
now have theprotobufBinding
function to check the request format, but didn't have a protobuf response function likec.YAML()
.In our company ByteDance, the largest internet company using golang in China, we use
gin
to transfer Protobuf instead of Json, we have to write some internal library to make some wrappers to achieve that, and the code is not elegant. So we really want such a feature.