Use jsoniter instead of easyjson #2105
Conversation
openshift-ci-robot
added
the
size/XXL
|
Changes LGTM, however given it's size, should it wait until after the 1.0 release? |
|
MarshalIndent really hates using tabs as indents. Replaced with 5 spaces. @TomSweeneyRedHat Sure, we should probably delay until after we branch. |
|
all the changes are coming from vendored code. Even if the PR size is XXL, I think it is a safe change on our side. I think it should be OK to have it for 1.0 |
|
/lgtm |
|
☔ The latest upstream changes (presumably #2061) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Rebased. I'm still a little hesitant about having this in 1.0 - if there are compatibility issues with easyjson, we might have issues accessing databases created with sub-1.0 versions. My initial testing hasn't noticed anything like that, though. I'll slap a /approve on, as this should be ready now, 1.0 questions aside /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
bot, retest this please |
|
Not including this in 1.0 |
|
bot, retest this please |
mheon
force-pushed
the
jsoniter
branch
2 times, most recently
from
39a6a65 to
b66f2ba
Compare
|
☔ The latest upstream changes (presumably #2126) made this pull request unmergeable. Please resolve the merge conflicts. |
The jsoniter library does not require code generation, which is a massive advantage over easyjson (it's also about the same in performance). Begin moving over to it by removing the existing easyjson code. Signed-off-by: Matthew Heon <[email protected]>
We already have it vendored for a Kube package we import, but we want a more recent version with additional bugfixes over the 1.0 release we originally had. Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
The vndr tool isn't updating vendor.conf so do it manually. Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
The json-iterator package will panic on attempting to use MarshalIndent with a non-space indentation. This is sort of silly but swapping from tabs to spaces is not a big issue for us, so let's work around the silly panic. Signed-off-by: Matthew Heon <[email protected]>
|
@rhatdan @baude @vrothberg This is unblocked now that 1.0 is merged, we're good to merge this |
|
I think lint is a flake |
|
Let's get this merged. After that I may need to rebase the vendor-* PR. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Just as fast, no code generation involved. Theoretically both are fully compatible with the standard library encoding/json implementation.