Skip to content
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

CHANGELOG for JSON refactor + added back the `encode_big_decimal_as_string` option with warning #13060

Merged

Conversation

@chancancode
Copy link
Member

@chancancode chancancode commented Nov 26, 2013

/cc @jeremy

@jeremy
jeremy reviewed Nov 27, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
"The JSON encoder in Rails 4.1 no longer supports encoding BigDecimals as JSON numbers. " \
"If your application depends on this, you should install the 'activesupport-json_encoder' " \
"gem to restore the old behavior. In the future, the related configuration option, " \
"ActiveSupport.encode_big_decimal_as_string, will be removd from Rails."

This comment has been minimized.

@jeremy

jeremy Nov 27, 2013
Member

removed

This comment has been minimized.

@chancancode

chancancode Nov 27, 2013
Author Member

fixed, also changed "install the xxx gem" => "add the xxx gem to your Gemfile"

@carlosantoniodasilva
carlosantoniodasilva reviewed Nov 27, 2013
View changes
activesupport/CHANGELOG.md Outdated

The related configuration `ActiveSupport.encode_big_decimal_as_string` is deprecated in core
to ensure a smooth upgrade path. Reading from this option will always return true. Reading the
option or setting it to true will trigger a depreactation warnning. Setting it to false will

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 27, 2013
Member

depreactation warnning => deprecation warning

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 27, 2013
Member

Also it might be good to wrap true/false in backticks for highlighting.

@chancancode
chancancode reviewed Nov 27, 2013
View changes
activesupport/test/json/encoding_test.rb Outdated
@@ -172,6 +172,24 @@ def test_wide_utf8_roundtrip
assert_equal "

This comment has been minimized.

@chancancode

chancancode Nov 27, 2013
Author Member

Oops, will fix the name on next push

@carlosantoniodasilva
carlosantoniodasilva reviewed Nov 27, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
"The JSON encoder in Rails 4.1 no longer supports encoding BigDecimals as JSON numbers. " \
"If your application depends on this, you should add the 'activesupport-json_encoder' " \
"gem to your Gemfile to restore the old behavior. In the future, the related configuration " \
"option, ActiveSupport.encode_big_decimal_as_string, will be removed from Rails."

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 27, 2013
Member

How about:

The JSON encoder in Rails 4.1 no longer supports encoding BigDecimals as JSON numbers.
If your application depends on this, you should add the 'activesupport-json_encoder'
gem to your Gemfile to restore the old behavior, otherwise you can safely remove the 
related 'active_support.encode_big_decimal_as_string' configuration from your application.

So that we tell people what to do in all cases, wdyt?

This comment has been minimized.

@chancancode

chancancode Nov 27, 2013
Author Member

👍

@chancancode
Copy link
Member Author

@chancancode chancancode commented Nov 27, 2013

Updated with the suggestions from @carlosantoniodasilva. I kept the changelog entry in its original position when rebasing, partly because that's the order we merged these commits, and partly because I don't have to compete with future additions to the changelog :)

@carlosantoniodasilva
carlosantoniodasilva reviewed Nov 27, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
def encode_big_decimal_as_string=(as_string)
message = \
"The JSON encoder in Rails 4.1 no longer supports encoding BigDecimals as JSON numbers. Instead, " \
"the new enocder will always encode them as strings.\n\n" \

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 27, 2013
Member

encoder (in both messages)

This comment has been minimized.

@chancancode

chancancode Nov 27, 2013
Author Member

Thanks!

@carlosantoniodasilva
carlosantoniodasilva reviewed Nov 27, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
"the new enocder will always encode them as strings.\n\n" \
"You are seeing this error because you have 'active_support.encode_big_decimal_as_string' in " \
"your configuration file. If you have been setting this to true, you can safely remove it from " \
"your configuration. Otherwise, you should add the 'activesupport-json_encoder' gem to your " \

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 27, 2013
Member

two spaces after gem

This comment has been minimized.

@chancancode

chancancode Nov 27, 2013
Author Member

Thanks!

@carlosantoniodasilva
carlosantoniodasilva reviewed Nov 30, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
"the new encoder will always encode them as strings.\n\n" \
"You are seeing this error because you are trying to check the value of the related configuration, " \
"'active_support.encode_big_decimal_as_string'. For now, this option will always be true. In the " \
"future, it will be removed from Rails, so you should stop checking its value."

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Nov 30, 2013
Member

I think it might be interesting to talk about the gem here too? If people are checking this, they might have a reason for doing so?

This comment has been minimized.

@chancancode

chancancode Nov 30, 2013
Author Member

Good call, updated

@carlosantoniodasilva
carlosantoniodasilva reviewed Dec 3, 2013
View changes
activesupport/lib/active_support/json/encoding.rb Outdated
"The JSON encoder in Rails 4.1 no longer supports encoding BigDecimals as JSON numbers. Instead, " \
"the new encoder will always encode them as strings.\n\n" \
"You are seeing this error because you are trying to check the value of the related configuration, " \
"'active_support.encode_big_decimal_as_string'. If your application depend on this option, you should " \

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Dec 3, 2013
Member

If your application depends

@carlosantoniodasilva
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva commented Dec 3, 2013

Minor comment, I think we're good to :shipit:

Also added the missing CHANGELOG entry for #12183 @ 80e7552 and
4d02296.
@chancancode
Copy link
Member Author

@chancancode chancancode commented Dec 3, 2013

carlosantoniodasilva added a commit that referenced this pull request Dec 3, 2013
CHANGELOG for JSON refactor + added back the `encode_big_decimal_as_string` option with warning
@carlosantoniodasilva carlosantoniodasilva merged commit 6e905e2 into rails:master Dec 3, 2013
@carlosantoniodasilva
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva commented Dec 3, 2013

Thanks ❤️

@chancancode
Copy link
Member Author

@chancancode chancancode commented Dec 3, 2013

😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.