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

Fix RubyMoney/Money i18n deprecation warning #2891

Conversation

jacobherrington
Copy link
Contributor

This PR fixes the warning message coming from RubyMoney/money discussed in RubyMoney/money#816

[DEPRECATION] `use_i18n` is deprecated - use `Money.locale_backend = :i18n` instead

It might be worth waiting to see if this is fixed in the money gem before we merge this PR.

@jacobherrington
Copy link
Contributor Author

jacobherrington commented Oct 4, 2018

😭
screen shot 2018-10-04 at 4 30 12 pm

@kennyadsl
Copy link
Member

kennyadsl commented Oct 8, 2018

As far as I get from this comment in the issue on RubyMoney/money, you have to set that configuration up in each application. I'm not sure that initializer works but setting that here it removes the deprecations in specs. I think we need to put that setting in a place where it is also configurable by users with the other Spree::Money configurations.

@jacobherrington
Copy link
Contributor Author

That might be a better solution, although this does fix it when I run tests which was what I was trying to fix. I'll try to make it a configuration option instead though.

@tvdeyen
Copy link
Member

tvdeyen commented Oct 10, 2018

I think we need to put that setting in a place with is also configurable by users with the other Spree::Money configurations.

Yes, I agree. There is also money-rails that has an install generator, that creates the above mentioned initializer in the host app. Although I don't like to add another dependency, I think we should at least consider it or implement our own generator during Solidus install.

@jacobherrington
Copy link
Contributor Author

Work is being done on #2912

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

Successfully merging this pull request may close these issues.

None yet

3 participants