-
Notifications
You must be signed in to change notification settings - Fork 899
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
Upgrade i18n version #22550
Upgrade i18n version #22550
Conversation
ok, I thought someone already merged ruby-i18n/i18n#658 but it isn't even a PR yet. Lets hold off on this one until that is fixed Maybe we need to ensure we are using grosser/gettext_i18n_rails#196 |
update:
|
f642102
to
5aa9478
Compare
I figured I'd benchmark this: tl;dr I believe a case-insensitive regex is fastest, even though it's slightly slower in a particular error case. def test_start_with?(result)
result && !result.start_with?("Translation missing:") && !result.start_with?("translation missing:")
end
def test_regex_case_sensitive(result)
result && !result.match?(/^[Tt]ranslation missing:/)
end
def test_regex_case_insensitive(result)
result && !result.match?(/Translation missing:/i)
end
CASES = [
["translation missing: foo", false],
["Translation missing: foo", false],
[nil, nil],
["Other", true],
]
CASES.each do |result, expectation|
raise "test_start_with? for #{result.inspect} is bad." if test_start_with?(result) != expectation
raise "test_regex_case_sensitive for #{result.inspect} is bad." if test_regex_case_sensitive(result) != expectation
raise "test_regex_case_insensitive for #{result.inspect} is bad." if test_regex_case_insensitive(result) != expectation
end
CASES.each do |result, expectation|
puts "==================="
puts "Testing #{result.inspect}..."
puts
Benchmark.ips do |x|
x.report("start_with?") {
test_start_with?(result)
}
x.report("regex case sensitive") {
test_regex_case_sensitive(result)
}
x.report("regex case insensitive") {
test_regex_case_insensitive(result)
}
end
end
|
BTW, I don't strongly care one way or another and won't hold up merge for that - just let me know what you'd prefer |
missing translation: changed to Missing translation in the source Changed our dictionary to handle this case Support missing translations for i18n 1.14 i18n gem used to return "missing translation:" As of 1.14, it returns "Missing translation:" Changed our dictionary to handle this case
update:
Thnx for challenging me on this one |
Checked commits kbrock/manageiq@b4c7969~...1dc4bd9 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
Overview
i18n version 1.14 broke our build.
Temporary Solution
We had pegged i18n to version 1.13
Permanent Solution (this PR)
This reverts commit 6e11232, reversing changes made to b4c7593.
We then added the 3 following fixes to resolve issues of i18n v 1.14
For that last one, we had 2 options:
>=1.14
and have a single string comparison.We chose to handle both string comparisons. (thank you Jason for making sure we got the best performing code)