learningequality / ka-lite

KA Lite: lightweight web server for serving core Khan Academy content (videos and exercises) without needing internet connectivity
https://learningequality.org/ka-lite/
Other
456 stars 306 forks source link

[WIP] Stop unpacking .po files from content packs #5524

Closed mrpau-eugene closed 6 years ago

mrpau-eugene commented 6 years ago

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

Issues addressed

5518

codecov[bot] commented 6 years ago

Codecov Report

Merging #5524 into 0.17.x will increase coverage by 0.02%. The diff coverage is 15.15%.

Impacted file tree graph

@@            Coverage Diff            @@
##           0.17.x   #5524      +/-   ##
=========================================
+ Coverage   62.77%   62.8%   +0.02%     
=========================================
  Files         117     117              
  Lines        6555    6549       -6     
=========================================
- Hits         4115    4113       -2     
+ Misses       2440    2436       -4
Impacted Files Coverage Δ
kalite/i18n/management/commands/makemessages.py 95.83% <ø> (ø) :arrow_up:
...nagement/commands/setup_in_context_translations.py 0% <0%> (ø) :arrow_up:
...tributed/management/commands/contentpackchecker.py 0% <0%> (ø) :arrow_up:
kalite/i18n/base.py 55.15% <10.52%> (-0.7%) :arrow_down:
kalite/legacy/i18n_settings.py 86.36% <100%> (ø) :arrow_up:
...ributed/management/commands/retrievecontentpack.py 51.48% <11.11%> (+3.3%) :arrow_up:
kalite/coachreports/models.py 84.95% <0%> (-0.89%) :arrow_down:
kalite/project/settings/base.py 87.38% <0%> (-0.69%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f862026...35a4315. Read the comment docs.

benjaoming commented 6 years ago

Great that coverage will improve because affected (removed!) code has no coverage :D

mrpau-eugene commented 6 years ago

@benjaoming I just went back again on this PR, while reading the original issue again, I just wanted to know if the makemessages command should point to kalite/project/locale and not kalite/locale anymore?

mrpau-eugene commented 6 years ago

Originally, the instructions are:

But there were other factors that affects the old process and I cannot just simply remove the ~/.kalite/locale because there are some files being stored in there such as the {lang}_metadata.json, so I was reading the codebase and initially came up with these new procedures:

I’m not quite sure if it is correct and it still might be missing something. Mind checking these procedures @benjaoming ?

benjaoming commented 6 years ago

Hi @mrpau-eugene

But there were other factors that affects the old process and I cannot just simply remove the ~/.kalite/locale because there are some files being stored in there such as the {lang}_metadata.json

Ah no, the description doesn't say to remove the directory, just to remove it from the settings so we are sure translations still living there (from old content pack installations) are ignored:

Remove old functionality: Remove ~/.kalite/locale from LOCALE_PATHS, it shouldn’t be necessary anymore

benjaoming commented 6 years ago

Hi @mrpau-eugene - I'll try to continue work over in #5537, will perhaps pass it back to you later -- the communication regarding how to understand the issue that I posted was a bit heavy for me to keep up with, but the task is also very much entangled in our old understanding of how KA Lite works -- so hopefully this will be the last headache about translations stuff, before we can move on to a more modern CrowdIn-based approach.

mrpau-eugene commented 6 years ago

@benjaoming sure! just ping me anytime :)