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 I18n.t when locale contains separator #656

Merged
merged 6 commits into from
May 31, 2023

Conversation

tubaxenor
Copy link
Contributor

v1.13.0 broke a backward compatibility when locale contains separator, example:

# v1.12.0
require "i18n"

I18n.enforce_available_locales = false
I18n.backend.store_translations(:"en-api", v1: {foo: 'Foo in API'})
I18n.locale = :"en-api.v1"
I18n.t("foo")
# => "Foo in API"

# v1.13.0
require "i18n"

I18n.enforce_available_locales = false
I18n.backend.store_translations(:"en-api", v1: {foo: 'Foo in API'})
I18n.locale = :"en-api.v1"
I18n.t("foo")
# => "translation missing: en-api.v1.foo"

This PR brings back the separator separation for locale in normalize_keys.

@ngan
Copy link

ngan commented May 9, 2023

👋 @radar -- this bug is prevent us from updating to i18n 1.13.0. Anything we can do to help move this along? 🙇

@radar
Copy link
Collaborator

radar commented May 31, 2023

Apologies for the delay on this one, it's been a super-busy time of year for me.

@tubaxenor @ngan the build is failing here -- it appears the fix isn't going to work. Could you please investigate a fix that has all the tests passing?

You can run them with bundle exec rake test locally.

@ngan
Copy link

ngan commented May 31, 2023

Hi @radar, thanks for the response. main looks like it's failing a lot too: https://github.com/ruby-i18n/i18n/actions/runs/4807180573/jobs/8555607559

Just so we can debug this, should we be fixing the errors on main too? Or are they known/expected failures?

@radar radar mentioned this pull request May 31, 2023
2 tasks
@radar
Copy link
Collaborator

radar commented May 31, 2023

Thank you. I've fixed up some of those in the #661 PR. I would suggest using that as a base for your work. If I pull your commit in there, it will break what is currently a passing build. Could you please investigate how your change breaks that build?

@tubaxenor
Copy link
Contributor Author

@radar I've fixed the tests by 4aec070 🙇

@tubaxenor
Copy link
Contributor Author

@radar Looks like most of the remaining errors are the same as #661, but let me know if there are any remains from my PR 🙇

@radar radar merged commit 71400f6 into ruby-i18n:master May 31, 2023
@radar
Copy link
Collaborator

radar commented May 31, 2023

Thank you @tubaxenor. The changes look good to me. I am not sure why the Rails main build is failing. I am guessing that is a temporary situation

@radar
Copy link
Collaborator

radar commented Jun 2, 2023

@fatkodima Just an FYI to you: this PR reverts part of your performance work from #651. Would you like to re-attempt that work for the next release?

@fatkodima
Copy link
Contributor

@radar I retested again - the reverted part does not make much difference, if any. The part that gave most performance improvement stays in the codebase. So I do not think this reverted part is worth reintroducing.

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.

4 participants