-
Notifications
You must be signed in to change notification settings - Fork 9.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
Replace ugorji/codec with json-iterator/go #10667
Conversation
95d7354
to
09aebaf
Compare
@dims I don't see any failure related to your changes looking at quickly. Lately, CI is flakey. I will be reviewing the changes too. Thanks! |
8f7131f
to
4514211
Compare
github.com/ugorji/go is still listed in go.sum and is vendored... did you run |
@liggitt unfortunately the |
Codecov Report
@@ Coverage Diff @@
## master #10667 +/- ##
==========================================
- Coverage 71.63% 71.51% -0.12%
==========================================
Files 393 394 +1
Lines 36629 36658 +29
==========================================
- Hits 26238 26217 -21
- Misses 8551 8590 +39
- Partials 1840 1851 +11
Continue to review full report at Codecov.
|
@liggitt cleaned up as well! dropped 45k+ LOC |
@dims Could you update the commit title to |
@gyuho ack will fix the commit message shortly |
3110467
to
b7b212d
Compare
We need to use the stdlib-compatible one that is case-sensitive, etc Change-Id: Id0df573a70e09967ac7d8c0a63d99d6a49ce82f1
Change-Id: Iac4601443bcad71920fd96b97bfe21c16116577a
Change-Id: I1f3fc00f95efadd6da9b4c248156f8460ae0ff97
Change-Id: Ibfa24e28cacd58388f7606a945c8ac35e1c34580
…ugorji Using lessons learned from k8s changes: kubernetes/kubernetes#65034 Change-Id: Ia17a8f94ae6ed00c5af2595c2b48d3c9a0344427
b7b212d
to
3655a4b
Compare
@dims Sure, if it doesn't break compatibility. Would anything change in |
@dims aha..all green checks, looks good :-) |
@gyuho exact same code in this PR applies cleanly to 3.3-release (except for the vanity url). So we should be good |
Kubernetes removed the usage for ugorji in the following PR:
Use json-iterator instead of ugorji for JSON. #48287
However since etcd still uses this library, we end up increasing the size of the binaries in kubernetes by a lot. See:
kubernetes/kubernetes#76506
Can we please switch over to json-iterator/go as well to eliminate the unnecessary code/size?
Change-Id: Ia8b267e9ada49b22a1cd3bd7c05450e32582dc91
Please read https://github.com/etcd-io/etcd/blob/master/CONTRIBUTING.md#contribution-flow.