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

Switch to I18n::Backend::Simple.new due to i18n-js compatibility #106

Closed
wants to merge 1 commit into from

Conversation

kennyadsl
Copy link
Member

This will fix fnando/i18n-js#344

@jhawthorn
Copy link
Contributor

Is it possible to remove this file entirely? I don't think we should be doing what it does, and it seems like setting I18n.backend = I18n::Backend::Simple.new would bypass its functionality entirely anyways.

@kennyadsl
Copy link
Member Author

👍 We can probably revert this whole commit: 1bec159. I think i18n_data would be useless as well.

@jhawthorn
Copy link
Contributor

jhawthorn commented Jan 26, 2018

I think unfortunately this is being used by BaseHelper's available_countries. I still want to remove this file but I think we need to find another way to get translated country names.

@jhawthorn
Copy link
Contributor

Amusingly, this doesn't check the scope of the translation:

I18n.t('CA', scope: 'foo.bar.baz') #=> "Canada"

@jhawthorn
Copy link
Contributor

jhawthorn commented Feb 6, 2018

#2537 solves this and is ready to go, but I'm not sure how to handle this version-wise.

If we remove this initializer we'll be dropping support for translating countries in all versions < 2.5. Maybe we need a major version bump, to make this solidus_i18n 2.0.

@kennyadsl
Copy link
Member Author

I've prepared a PR (#108) to remove this dependency if we go for the major version bump.

Alternatively we could leave the initializer and conditionally load that code only for some solidus version using this check?

@tvdeyen tvdeyen added this to the 2.0 milestone Feb 7, 2018
@jhawthorn jhawthorn mentioned this pull request Apr 6, 2018
4 tasks
@jhawthorn
Copy link
Contributor

Solved by #122 🎉

@jhawthorn jhawthorn closed this May 2, 2018
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.

undefined method `initialized? for backend:chain
4 participants