-
Notifications
You must be signed in to change notification settings - Fork 69
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
fix: don't use integer for enums in json encoding #761
Conversation
Codecov Report
@@ Coverage Diff @@
## master #761 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 27 27
Lines 1619 1619
Branches 328 328
=========================================
Hits 1619 1619 Continue to review full report at Codecov.
|
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.
Integers for enums in json encoded requests is not desired.
Out of curiosity is there a doc somewhere stating this? Is it for human readability?
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.
Thanks for doing this!
If there is already a unit test that checks the REST request sent out, it would be good to capture this there. But if there isn't, don't worry about it right now; we'll do it later.
@busunkim96 Yes, the internal design docs specify this. Essentially, we can't guarantee that the numeric values will be preserved when converting from the Discovery file, so we use the string representations. |
Integers for enums in json encoded requests is not desired.