-
-
Notifications
You must be signed in to change notification settings - Fork 337
Add radix property to JsonFormat (#320) #321
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
base: 2.x
Are you sure you want to change the base?
Conversation
We have one potential problem here: there's no real "default" marker here, so any use of
which becomes problem when using global or per-type defaults. So I think it is necessary to instead use "magic constant" to indicate "use default"; |
Wouldn't we be fine with 10 being a default? Radix only becomes relevant when we are serializing a |
@tiger9800 Problem is not direct |
Oh I see, we can put a global config override of 16 and then apply an annotation Great catch, thank you. Will fix! |
@tiger9800 Exactly! We have had this challenge for some annotations before which is why I recognized it :) |
Added the marker default value. For translating from raw value, we have |
public boolean hasNonDefaultRadix() { | ||
return _radix != DEFAULT_RADIX && _radix != 10; |
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.
Should not check against 10
as ObjectMapper may specify other default radix
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.
Close; left couple of notes for changes.
A unit test failure wrt String representation. |
To implement FasterXML/jackson-databind#5317, we would like to use radix property of the @JsonFromat annotation. We make sure to add it to JsonFormat.Value to parallel the behavior of other properties of the @jsonformat annotation.