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

use native json marshaling to map stubbed output #52

Merged
merged 3 commits into from
Aug 3, 2020

Conversation

vincent-raman
Copy link
Contributor

@vincent-raman vincent-raman commented Jun 3, 2020

When an output field had a '_' in its name, it was not mapped
correctly by 'mapstructure' to the protoc generated class because,
in this class, the fields are translated in CamelCase.
Hower, the fields have json tag with their original name.
Json marshalling is then the way to fix this issue.

Fixes tokopedia/gripmock/#46

When an output field had a '_' in its name, it was not mapped
correctly by 'mapstructure' to the protoc generated class because,
in this class, the fields are translated in CamelCase.
Hower, the fields have json tag with their original name.
Json marshalling is then the way to fix this issue.
@vincent-raman vincent-raman marked this pull request as ready for review June 3, 2020 06:03
@jekiapp
Copy link
Contributor

jekiapp commented Jun 25, 2020

Thanks @vincent-raman for the PR.
Could you please also make a testcase for it. A simple update for the current example testcase is okay, just to prove the issue is solved. Please check /example folder for the test cases.

@vincent-raman
Copy link
Contributor Author

Hi, I modified the basic example, does it seem ok for you ?

@jekiapp jekiapp merged commit 398c134 into tokopedia:master Aug 3, 2020
@jekiapp
Copy link
Contributor

jekiapp commented Aug 3, 2020

LGTM! thanks

@bjaglin
Copy link

bjaglin commented Oct 30, 2020

I ran into this today, as it introduced a regression for my stubs generated with https://github.com/scalapb/scalapb-json4s. What's surprising me is that the canonical proto3 encoding defaults to lowerCamelCase, so the previous behavior seemed more correct than the one introduced in this PR?

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

Successfully merging this pull request may close these issues.

3 participants