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

Use locale name instead of language name #163

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

morenol
Copy link
Contributor

@morenol morenol commented Oct 11, 2019

There is a problem with the translations (see #159)

The translations catalog(ubcpi/static/js/src/translations.js) is generated using locale names and translations from transifex.

The django.utils.translation.get_language() function returns the language code (i.e. es-419), but since in the catalog we are using locale names(i.e. es_419), the strings are not being translated as expected.

With this change, now we are using the locale names and it works fine. I tested this with spanish

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 96.715% when pulling 8770d2e on eduNEXT:lmm/translations into 998a52f on ubc:master.

@morenol
Copy link
Contributor Author

morenol commented Oct 18, 2019

Hello @kitsook, Can you review this?

@kitsook
Copy link
Contributor

kitsook commented Oct 24, 2019

Hi @morenol , thanks for submitting this pull request. Give me some time to check what is the best way to handle the translations for openedx.

As for #159, we probably shouldn't edit translations.js directly and the file is generated dynamically.

ubcpi/Makefile

Lines 58 to 64 in 998a52f

compile:
for i in `find ubcpi/translations/ -name *.po`; do msgfmt $$i -o `dirname $$i`/text.mo; done
node_modules/.bin/angular-gettext-cli --compile --files 'ubcpi/translations/**/*.po' --format javascript --dest 'ubcpi/static/js/src/translations.js'
pull_translations:
tx pull --force --mode=reviewed -l=ar,es_419,ja_JP,fr,fr_CA,he,hi,ko_KR,pt_BR,ru,zh_CN,de_DE,pl
make compile

I wonder if we should also change the tx pull --force --mode=reviewed -l=ar,es_419... line accordingly

@morenol
Copy link
Contributor Author

morenol commented Oct 25, 2019

Hello @kitsook, I tried to change that line but it didn't work because the transifex project uses also the locale name: https://www.transifex.com/open-edx/xblocks/ubcpi-peer-instruction/

For that reason I used this approach.

On the other hand, I think that we should change the line:

tx pull --force --mode=reviewed -l=ar,es_419,ja_JP,fr,fr_CA,he,hi,ko_KR,pt_BR,ru,zh_CN,de_DE,pl 

with something that can pull all the languages from transifex without restriction.

In the edx-ora2 project it is not being used the -l paramater, instead they use the --minimum-perc parameter to pull all the translation from the languages that have at least 1% of reviewed translations.

https://github.com/edx/edx-ora2/blob/283776514efc1940c9c98d756cba4590607a13cd/Makefile#L60

@kitsook
Copy link
Contributor

kitsook commented Oct 29, 2019

@xcompass I would suggest to merge this PR instead of #159. transifex is using locale codes to identify translations. This change is more appropriate than modifying individual translation packages and translations.js directly.

As for modifying tx pull... to get all the translations, we can further discuss the criteria and review rate etc.

@morenol
Copy link
Contributor Author

morenol commented Nov 28, 2019

@kitsook Any updates on this?

If we run the command to update the translations, the changes applied in the last PR merged will disappear.

@kitsook kitsook merged commit 87c148b into ubc:master Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants