open-craft / xblock-poll

An XBlock for Polling users and displaying the result
GNU Affero General Public License v3.0
18 stars 66 forks source link

[BB-3673] Recompile translations and cleanup translations folder #89

Closed giovannicimolin closed 3 years ago

giovannicimolin commented 3 years ago

Recompile translations and cleanup translations folder. This fixes an issue with some case insensitive filesystems, which made it impossible to clone the repo.

See https://openedx.slack.com/archives/C01ATGRBBDM/p1612298099047900 for context.

Testing instructions:

  1. Install block.
  2. Check that translations are working as expected.
  3. Check that there are no case-insensitive name conflicts in the translation folder.

Not sure if removing the files broke the translations (all new ones where automatically generated).

Reviewer:

bradenmacdonald commented 3 years ago

@giovannicimolin

Check that translations are working as expected.

What's the easiest way to do this?

which made it impossible to clone the repo.

It seems I can still clone the repo, but it shows changes to six files like poll/public/js/translations/de_DE/textjs.js immediately after cloning. And it's then super buggy - I can't change branches, commit the changes, reset the changes, or do anything really.

I actually can't even checkout this branch to test it!

I had to delete the repo and then clone it with git clone --branch giovanni/fix-translations git@github.com:open-craft/xblock-poll.git. I can confirm that that works at least.

Check that there are no case-insensitive name conflicts in the translation folder.

OK, done - I can confirm this branch fixes that. 👍🏻

kaizoku commented 3 years ago

@giovannicimolin I can't very adequately test the case insensitivity issues on a linux machine. I tried using ext4's case insensitive option, but my I'm missing some kernel options to use them. I could set those up, but since @bradenmacdonald has tested this from OS X that's probably more representative of the platform where most people are probably having issues from that.

I tested the translations with this current PR (though not for every single language), but they're applying just fine for Arabic, German, French, and Japanese.

giovannicimolin commented 3 years ago

@bradenmacdonald @kaizoku Thanks for the reviews. I'll take your comments as an approval review.

What's the easiest way to do this?

Install the block in the devstack and then use http://localhost:18000/update_lang/ to test the translated language. @kaizoku Already did it for some languages, and I checked a few others too, so no need to follow up on this one unless you think it might cause some issue on your Mac.

giovannicimolin commented 3 years ago

Closing comment: I've merged this with only the Python 3.8 checks passing. The code makes use of f-strings which aren't available in older python versions.

I'll schedule a follow up to fix the CI.