clemente
commented
4 years ago @pkulkark I read the code.
I see that this PRs includes these changes:
- The main goal is to change text.po and text.mo in Hebrew
- This PR is also changing textjs.js for all languages
- In addition it's compiling text.mo for many (not all) languages
text.po in Hebrew has been changed to include the 2 new strings :ok: . They are in the .mo too.
The changes have been done to poll/translations/he/LC_MESSAGES/text.po
but not to public/js/translations/he/text.js :warning:
It looks like all messages from text.po should be present in text.js too (even if they aren't used because they're Python only).
I think we should include them, if possible.
The updates to the non-Hebrew language in textjs.js seem to be automatic and include:
- removing whitespace from end of lines, for instance in
"Answer": "Antwort",in German. Ok - removing quotes around numbers. e.g.
"FIRST_DAY_OF_WEEK": "0",→"FIRST_DAY_OF_WEEK": 0,. I'm not sure why, but it makes sense to trust this autogenerated output as long as things keep working - changing a line:
return value[django.pluralidx(count)];→return value.constructor === Array ? value[django.pluralidx(count)] : value;. The new version looks safer and is backwards-compatible - so I consider the
textjs.jschanges ok :ok: - I don't know why many of the changes happened and it would be safe to ignore them or exclude them from this PR too
Changes to text.mo (which was compiled for many languages) involve just removing a header line "POT-Creation-Date: 2018-09-03 09:32+0000\n", but no other changes (in particular: no string translations), except for Hebrew, where the 2 new messages were added. I saw this by decompiling each one with msgunfmt and comparing them. So the .mo are safe. :ok:
The PR modifies, for instance, ja_JP/textjs.js (in poll/public/js/translations). However, there are parallel files ja-jp/textjs.js and ja_jp/textjs.js (in addition to ja_JP/textjs.js).
It looks like they should be copy-pasted to have the same contents. Older PRs like https://github.com/open-craft/xblock-poll/pull/65 show that all files must be updated in parallel.
The changes to textjs.js have been neutral but there have been changes, as mentioned above, and we would be safer if all the textjs.js for each language match. :warning: So could you copy your changes so that the textjs.js match?
It looks like the duplicate/triplicate languages are: de-de de_de de_DE, en-us en_us, es-419 es_419, fr_ca fr_CA, ja-jp ja_jp ja_JP, ko_kr ko_KR, pt-br pt_br pt_BR, zh-cn zh_cn zh_CN. I hope we can fix this some time (maybe by using links instead of copy-pasting), but we don't have time in this PR.
Finally, I have seen that even small changes like https://github.com/open-craft/xblock-poll/pull/65 have bumped up the version number. Could we make this here too, so that other users don't see changes until they upgrade?
I tried to test this in Campus stage, but there's a [known] bug in Studio and I can't add a component to a unit, so I can't add a poll. Is there an existing poll to test this? And do you have more details about that bug?
pkulkark
This PR adds new hebrew translation strings for "Submit" and "View results".
Jira Tickets: SE-3253
Testing Instructions: TBD
Reviewers: