-
-
Notifications
You must be signed in to change notification settings - Fork 410
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
Introduce support for deprecating translation keys #490
base: master
Are you sure you want to change the base?
Conversation
981b12d
to
c229b54
Compare
Adds an `I18n::deprecate` API that allows users to mark a key as deprecated and superseded by another. `I18n::t` will then use the following logic: - If the key supersedes a deprecated one, and the old one is still defined, use the old one instead. If the old one is not defined anymore, use the new one. - If the key is deprecated and the old one is still defined, use the old one. If the old one is not defined anymore, use the new one. - For all other keys, regular behavior is maintained. The reason the old key is always preferred to the new one when available is that users may have been using the new key for other purposes, and we don't want to swap it for the old one without notice. Instead, users have to manually go in and delete the old key for the new one to start being used.
c229b54
to
9bcca2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one suggestion on how to print the deprecation 👍
new_key = I18n.config.deprecations[normalized_key] | ||
|
||
if new_key | ||
puts "DEPRECATION WARNING: #{normalized_key} is deprecated, use #{new_key} instead" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using warn
(printing to stderr) instead of puts, as it is more common for deprecations.
You might also want to use warn(…, uplevel: 1)
for rubies that support it, but you need to weight the checks on ruby version vs. the usability improvement of knowing when the error is originated.
Hi I like the idea to handle deprecation on translations. We can do it with this kind of implementation: module I18n
module Backend
class Deprecator
def store_translations(locale, data, options = EMPTY_HASH)
...
end
def available_locales
...
end
def lookup(locale, key, scope = [], options = EMPTY_HASH)
key = find_the_key_in_the_dedicated_store
Warning.warn("Deprecated key #{key}") if key
return nil # Always return nil to get the fallback on I18n::Backend::Simple or other backend in the chain.
end
end
end
end then This way, we won't change anything in |
This was born out of the discussion on solidusio/solidus_auth_devise#173. It was originally started by @kennyadsl and completed by me.
The rationale and logic are explained in the commit message for future reference.
Please note that it's very experimental and we understand it may be too complex and/or opinionated to be introduced into I18n directly. We're happy to accept any change requests or even to make it a separate gem if you feel like I18n is not a good place for it.
It's still a bit rough at the edges. If this sparks enough interest, we'd like to:
translate
patches toDeprecator
. This would be much cleaner, but it has some performance drawbacks, mainly that we'd need to calllookup
inDeprecator
just to check if the keys are defined, but then still delegate tosuper
for the rest of the logic, andsuper
would calllookup
again.Hopefully, the current state of it is enough for you to get an idea!