Changes serialization type from default to non-empty. #228
Conversation
…hey are non-empty instead of defaults.
thinkingserious
added
status: code review request
| SubscriptionTrackingSetting setting = new SubscriptionTrackingSetting(); | ||
| setting.setEnable(false); | ||
|
|
||
| String jsonTwo = mapper.writeValueAsString(setting); |
There was a problem hiding this comment.
Should be jsonOne. This is assuming that the numeral part of the variable name is the expected number of fields in the serialized version. If that's not the case, then can you elaborate on the naming convention?
There was a problem hiding this comment.
yeah, you are right, just a stupid copy-pasting issue ;)
| GoogleAnalyticsSetting setting = new GoogleAnalyticsSetting(); | ||
| setting.setEnable(false); | ||
|
|
||
| String jsonTwo = mapper.writeValueAsString(setting); |
There was a problem hiding this comment.
Should be jsonOne. This is assuming that the numeral part of the variable name is the expected number of fields in the serialized version. If that's not the case, then can you elaborate on the naming convention?
There was a problem hiding this comment.
see comment on top
| FooterSetting setting = new FooterSetting(); | ||
| setting.setEnable(false); | ||
|
|
||
| String jsonTwo = mapper.writeValueAsString(setting); |
There was a problem hiding this comment.
Should be jsonOne. This is assuming that the numeral part of the variable name is the expected number of fields in the serialized version. If that's not the case, then can you elaborate on the naming convention?
There was a problem hiding this comment.
see comment on top
| BccSettings settings = new BccSettings(); | ||
| settings.setEnable(false); | ||
|
|
||
| String jsonTwo = mapper.writeValueAsString(settings); |
There was a problem hiding this comment.
Should be jsonOne. This is assuming that the numeral part of the variable name is the expected number of fields in the serialized version. If that's not the case, then can you elaborate on the naming convention?
There was a problem hiding this comment.
see comment on top
|
LGTM |
|
@thinkingserious hi, how are we going to proceed with this one? |
|
LGTM Could you please resolve the merge conflict and I'll get this merged. With Best Regards, Elmer |
|
Yup, will do.
…Sent from my iPhone
On 31. Oct 2017, at 20:49, Elmer Thomas ***@***.***> wrote:
LGTM
Could you please resolve the merge conflict and I'll get this merged.
With Best Regards,
Elmer
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Hello @dmitraver, |
|
Hi! I'm having the problem this issue is fixing with the Setting.java class, was it left out on purpose? Can this same fix be added to that class? |
Fixes #216.
Settings will be serialized anyway even if no parameters are set and defaults are used.