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

I18n::Backend::Chain#translations should merge from all backends #470

Merged
merged 4 commits into from
Mar 10, 2019

Conversation

gburgett
Copy link
Contributor

https://github.com/fnando/i18n-js uses the #translations method to dump all translations to a javascript file during asset compilation. The chain backend should deep_merge all these translations in order to dump the appropriate values.

My use case:
I've created a backend which pulls translations from my CMS, and hooked that up in a chain backend to first try the CMS and second try the default .yml files. I18n-js was not dumping any of the translations from my yml files to the resulting javascript.

https://github.com/fnando/i18n-js uses the `#translations` method to dump all translations to a javascript file during asset compilation.  The chain backend should deep_merge all these translations in order to dump the appropriate values.
lib/i18n/backend/chain.rb Outdated Show resolved Hide resolved
@radar
Copy link
Collaborator

radar commented Feb 25, 2019

The intention behind this change is good. I think it is a sensible approach.

It breaks some tests, however.

Please look at CI to see what has broken (https://travis-ci.org/ruby-i18n/i18n/jobs/493813293) and work on fixing it.

@gburgett
Copy link
Contributor Author

gburgett commented Mar 7, 2019

Hi @radar ,

Please take another look.

@radar radar added this to the 1.7.0 milestone Mar 10, 2019
@radar
Copy link
Collaborator

radar commented Mar 10, 2019

This code looks great to me. Thanks @gburgett!

@radar radar merged commit 7bfc725 into ruby-i18n:master Mar 10, 2019
@vipera vipera mentioned this pull request Nov 12, 2019
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.

2 participants