Added runtime support for rust_decimal#470
Added runtime support for rust_decimal#470Isaac-Duarte wants to merge 6 commits intoAleph-Alpha:mainfrom
Conversation
|
@NyxCode I would love your review on this when you have time! Thanks for the collaboration! |
|
Thanks for the PR. This is exactly what I had in mind, very nice. When serializing, When deserializing, We could either go with your implementation, although the type ( I'm inclined to go with your current implementation, since that seems to me to be what most users would be expecting. |
gustavo-shigueo
left a comment
There was a problem hiding this comment.
I'm inclined to go with your current implementation, since that seems to me to be what most users would be expecting.
Any thoughts @gustavo-shigueo?
I don't fully understand how rust_decimal works tbh, but this implementation looks sensible
|
Thanks! I fixed the merge conflict. |
|
Do you think this can be merged now @NyxCode ? |
|
I fixed the test issues, they run clean on my end!! |
Goal
This pull request adds initial support for the
rust_decimalcrate.Note this does not support
serde(with=...feature flags from rust_decimal.As discussed with @NyxCode, we could utilize
serdeunder the hood to detect this at runtime. Due to having to write a serializer the code is a little but of a mouth full. Another alternative would be to useserde_json, but I think I'd rather support this feature viaserdeas opposed to having to depend onserde_jsonfor this feature.Closes #272
Changes
I implemented a probe serializer that will perform a one time (via
OnceLock) attempt to get the runtime serialization type of arust_decimal::Decimal.I had a hard time coming up with a way to test this feature for both serialization formats without adding an additional feature flag, so some input/feedback there would be appreciated!
Checklist
Sanity test
I ran a test using this branch.
The struct:
I attempted to run
cargo testwith this feature enabled.I got:
I attempted to run
cargo testwith this feature enabled.I got: