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

MBS-10579: Unbreak es-es/el-gr translations #1363

Merged
merged 1 commit into from Feb 3, 2020

Conversation

mwiencek
Copy link
Member

@mwiencek mwiencek commented Jan 24, 2020

https://tickets.metabrainz.org/browse/MBS-10579

The code and associated comment here don't make sense to me. The language used in the Jed files should match what's in DBDefs, since that's what gets set in the lang cookie and is what's used to fetch the file.

I'd also be in favor of changing these to es/el (without country codes) in MB_LANGUAGES in production and removing the symlinks - they were to work around a bug in Locale::Util that's been fixed for years. But in that case we'd still remove this code as unused.

The code and associated comment here don't make sense to me. The
language used in the Jed files should match what's in DBDefs, since
that's what gets set in the lang cookie and is what's used to fetch the
file.

I'd also be in favor of changing these to es/el (without country codes)
in MB_LANGUAGES in production and removing the symlinks - they were to
work around a bug in Locale::Util that's been fixed for years. But in
that case we'd still remove this code as unused.
Copy link
Member

@reosarevok reosarevok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd agree with just using es (we don't target any specific Spanish variant). I'm not 100% sure if that same is the case with Greek, but probably?

Anyway, for now this seems fine.

Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for having country-specific locales here? I thought that was due to Latin American Spanish being very different and widespread, but I have no clue about Greek either.

@mwiencek
Copy link
Member Author

What was the reason for having country-specific locales here?

They're actually es and el in Transifex. I mentioned why we have the symlinks in the commit message/PR description:

they were to work around a bug in Locale::Util that's been fixed for years

Basically, web_set_locale had a bug where it wouldn't use an es_ES locale on the system if you specified just es.

@yvanzo
Copy link
Contributor

yvanzo commented Jan 30, 2020

Thanks, I was just surprised it affected specifically these two languages and not every other (de, fr, …).

@mwiencek mwiencek merged commit 4013bf8 into metabrainz:master Feb 3, 2020
@mwiencek mwiencek deleted the mbs-10579 branch February 3, 2020 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants