Deprecate custom BigDecimal serialization#13911
Conversation
|
I haven't included tests in this PR because I'm not sure how you guys typically approach tests for a mere deprecation. Just let me know if there's something on that front I should do. |
|
|
|
This broke the whole test suite. Can you take a look? Also I think we should only deprecate the yaml conversion. Right now we are removing more than we need to. |
|
BTW, thanks @davidcelis for working on this |
|
I had removed the whole file due to the discussion in the linked Issue, but I can remove only the YAML serialization if that's what people decide is a better idea. |
|
We still need the custom So I think we should split that file in 2, deprecate the yaml serialization and leave the custom |
|
Gotcha. I'm assuming I should leave the |
|
We can remove the |
|
Great. I'll add a separate commit for that; I can squash it later if you want. |
There was a problem hiding this comment.
We can remove this require.
There was a problem hiding this comment.
Woops, thought I had; nice catch.
Rails currently provides an extension to BigDecimal that redefines how it is serialized to YAML. However, as noted in rails#12467, this does not work as expected. When ActiveSupport is required, BigDecimal YAML serialization does not maintain the object type. It instead ends up serializing the number represented by the BigDecimal itself which, when loaded by YAML later, becomes a Float: ```ruby require 'yaml' require 'bigdecimal' yaml = BigDecimal('13.37').to_yaml YAML.load(yaml).class require 'active_support/all' yaml = BigDecimal('13.37').to_yaml YAML.load(yaml).class ``` @tenderlove posits that we should deprecate the custom BigDecimal serialization and let Ruby handle it. For the time being, users who require this serialization for backwards compatibility can manually `require 'active_support/core_ext/big_decimal/yaml_conversions'`. This will close rails#12467 and deprecate the custom BigDecimal#to_yaml. Signed-off-by: David Celis <[email protected]>
This was backported for Ruby 1.8 support and is no longer needed. Signed-off-by: David Celis <[email protected]>
…tion Deprecate custom BigDecimal serialization Conflicts: activesupport/CHANGELOG.md
|
Thanks! |
3 participants
Rails currently provides an extension to BigDecimal that redefines how
it is serialized to YAML. However, as noted in #12467, this does not
work as expected. When ActiveSupport is required, BigDecimal YAML
serialization does not maintain the object type. It instead ends up
serializing the number represented by the BigDecimal itself which, when
loaded by YAML later, becomes a Float:
@tenderlove posits that we should deprecate the custom BigDecimal
serialization and let Ruby handle it. For the time being, users who
require this serialization for backwards compatibility can manually
require 'active_support/core_ext/big_decimal/conversions'. I wouldagree. So, remove
active_support/core_ext/big_decimalso that thisextension is no longer required along with
active_support/all.This will close #12467 and deprecate the custom BigDecimal#to_yaml.
Signed-off-by: David Celis [email protected]