Remove the sr locale override .rb files

[mjankowski]

History here:

2019-06 - Files added to Mastodon: #11061

Same Day - Details of the issue added on upstream: svenfuchs/rails-i18n#853

2021-05 - Upstream fix: https://github.com/svenfuchs/rails-i18n/pull/925/files

2022-01 - Upstream fix part two: https://github.com/svenfuchs/rails-i18n/pull/972/files

2022-02 - Upstream release with fixes: https://github.com/svenfuchs/rails-i18n/blob/master/CHANGELOG.md#702-2022-02-12

It looks like these files were only in place to nudge the pluralization rules, which have since been fixed in rails-i18n.

There were no specs added with the original change - #11061 - although there were subsequently specs added (around locale recognition, not correct pluralization) which do use sr-Latn. Because of that, I'm sort of taking the word of those PRs that this fixes the issue. Not totally necessary, but would be nice to add a spec here showing the behavior that our config was putting in place and double check that it still works now on newer rails-i18n version. Happy to do that if I can get some guidance on what was failing originally (looks like many pluralizations?). I did some locale console testing to be sure that some translations using counts in both locales still worked.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[ClearlyClaire]

Thanks for digging into this!

Note that this was supposedly fixed in rails-i18n 7.0.2, and we are still using 6.0.0, so the PR as is would reintroduce the issue.

[mjankowski]

Note that this was supposedly fixed in rails-i18n 7.0.2, and we are still using 6.0.0, so the PR as is would reintroduce the issue.

We'll that's amusing. Will update with version bump.

Any guess on a spec which would expose the regression?

[mjankowski]

We'll that's amusing. Will update with version bump.

Added the version bump, but hold off on merge, doing more research here.

[mjankowski]

Any guess on a spec which would expose the regression?

I looked into this more and can't figure out what spec to add to catch the regression.

The places which use the many: key in the actual translations file are:

• The datetime.distance_in_words scope
• The generic errors scope

It looks like at least for these locales (and it appears most/all others) we are overriding the datetime one with values that never hit the pluralization keys.

For the latter errors ones, both locales do have a many key (from the gem) defined, whose values are the same as other, which I'm assuming is a compatibility thing, and/or just wasn't cleaned up. If I just do some console work and translate errors.messages.too_long for example, the output looks like what I'd expect based on the locale files. There are not inflections defined for either locale, so I assume its not the pluralize/singularize route.

• The override was previously setting the locale pluralization to Romanian - https://github.com/svenfuchs/rails-i18n/blob/master/lib/rails_i18n/common_pluralizations/romanian.rb
• The upstream changes set it to use OneFewOther - https://github.com/svenfuchs/rails-i18n/blob/master/lib/rails_i18n/common_pluralizations/one_few_other.rb (renamed/generalized from Serbian at some point).
• These are not exactly the same as each other, but they are different in what I'm assuming are intentional ways. If I play with different count values I can get them to behave differently, bug again that's really targeting something just for the sake of finding a difference, not necessarily exposing an issue.

All of that is to say -- I suspect that removing these files along with the version bump of the gem is likely fine and will get the correct pluralization treatment; but without knowing the initial issue we were solving, I'm not totally sure.

@Gargron I realize this was ~4 years ago - but do you have any more specific memory of what the original thing being solved here was; or if not - do you have an opinion one way or the other on my summary of the history here?

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[mjankowski]

Rebased against main, which now includes the version bump already.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[ClearlyClaire]

Any guess on a spec which would expose the regression?

I suppose calling something like I18n.t('accounts.followers', count: 530, locale: :'sr-Latn') and checking the result would work, but it's too tied to actual translations for my liking.

The places which use the many: key in the actual translations file are:

* The `datetime.distance_in_words` scope

* The generic `errors` scope

I'm confused, I'm unable to find any of those uses.

[mjankowski]

I suppose calling something like I18n.t('accounts.followers', count: 530, locale: :'sr-Latn') and checking the result would work, but it's too tied to actual translations for my liking.

Yup, I had some hesitation on those -- but also, because I wasn't sure if the original issue was just a wrong translation or an error being raised or something else, I wasn't sure what to actually check there.

The places which use the many: key in the actual translations file are:

* The `datetime.distance_in_words` scope

* The generic `errors` scope

I'm confused, I'm unable to find any of those uses.

Datetime example that gets overridden and not actually pluralized - https://github.com/mastodon/mastodon/blob/main/config/locales/en.yml#L1092

The errors ones are in the gem -- https://github.com/svenfuchs/rails-i18n/blob/master/rails/locale/en.yml#L134