search

airbnb / polyglot.js

Give your JavaScript the ability to speak many languages.
http://airbnb.github.io/polyglot.js
BSD 2-Clause "Simplified" License
3.71k stars 208 forks source link

Fix Russian Logic & added LocalId for russian #115

Closed darewreck54 closed 6 years ago

darewreck54 commented 6 years ago

After talking to our translator, found a bug in the russian logic.

image

It's related to https://github.com/airbnb/polyglot.js/pull/113 since Serbian and Bosnian actually share the same form as Russian. Depending how you want to merge the PR, that one can just end up being adding the additional test and new language/localeId.

I created a separate PR just for the Russian fix.

darewreck54 commented 6 years ago

@ljharb Did you separate russia and craotia eventhough they have the same plural form?

ljharb commented 6 years ago

@darewreck54 i did; i think the clarity of having a separate function is better than avoiding the code repetition. We can always extract the common forms out to a helper function if needed.

darewreck54 commented 6 years ago

Oops, I actually changed it in this pull request. I can separate it later.

Sent from my iPhone

On Jun 28, 2018, at 4:00 PM, Jordan Harband notifications@github.com<mailto:notifications@github.com> wrote:

@darewreck54https://github.com/darewreck54 i did; i think the clarity of having a separate function is better than avoiding the code repetition. We can always extract the common forms out to a helper function if needed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/airbnb/polyglot.js/pull/115#issuecomment-401198022, or mute the threadhttps://github.com/notifications/unsubscribe-auth/APbCzr69UjCQ7jrpeOBmJ6HAVgsrDc8Vks5uBV_4gaJpZM4U0Pw9.

ljharb commented 6 years ago

@darewreck54 this one's already merged, and i changed it back :-) no worries.