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

mrpau-eugene · 2017-10-02T02:08:55Z

TODO

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

Remove old functionality: Stop unpacking .po files from content packs
Remove old functionality: Remove ~/.kalite/locale from LOCALE_PATHS, it shouldn't be necessary anymore
Ensure that new path kalite/project/locale works

Issues addressed

#5518

codecov · 2017-10-02T02:24:35Z

Codecov Report

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

@@            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% <ø> (ø)`	⬆️
...nagement/commands/setup_in_context_translations.py	`0% <0%> (ø)`	⬆️
...tributed/management/commands/contentpackchecker.py	`0% <0%> (ø)`	⬆️
kalite/i18n/base.py	`55.15% <10.52%> (-0.7%)`	⬇️
kalite/legacy/i18n_settings.py	`86.36% <100%> (ø)`	⬆️
...ributed/management/commands/retrievecontentpack.py	`51.48% <11.11%> (+3.3%)`	⬆️
kalite/coachreports/models.py	`84.95% <0%> (-0.89%)`	⬇️
kalite/project/settings/base.py	`87.38% <0%> (-0.69%)`	⬇️

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.

kalite/settings/base.py

-if not os.path.exists(DEFAULT_DATABASE_DIR):
-    os.mkdir(DEFAULT_DATABASE_DIR)
+
+ensure_dir(USER_WRITABLE_LOCALE_DIR)


benjaoming

Small changes :)

kalite/settings/base.py

 DEFAULT_DATABASE_DIR = os.path.join(USER_DATA_ROOT, "database",)
-if not os.path.exists(DEFAULT_DATABASE_DIR):
-    os.mkdir(DEFAULT_DATABASE_DIR)


kalite/settings/base.py

@@ -188,16 +188,15 @@
 # the user running kalite and should be in a user-data
 # storage place.

-USER_WRITABLE_LOCALE_DIR = os.path.join(USER_DATA_ROOT, 'locale')
-KALITE_APP_LOCALE_DIR = os.path.join(USER_DATA_ROOT, 'locale')
+from fle_utils.general import ensure_dir


benjaoming · 2017-10-04T12:39:11Z

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

kalite/distributed/management/commands/retrievecontentpack.py

+    lang = lcode_to_django_lang(lang)
+    modir = get_po_filepath(lang)
+    if not os.path.exists(modir):
+        os.makedirs(modir)


kalite/settings/base.py

@@ -188,14 +188,12 @@
 # the user running kalite and should be in a user-data
 # storage place.

-USER_WRITABLE_LOCALE_DIR = os.path.join(USER_DATA_ROOT, 'locale')
-KALITE_APP_LOCALE_DIR = os.path.join(USER_DATA_ROOT, 'locale')
+USER_WRITABLE_LOCALE_DIR = os.path.join(USER_DATA_ROOT, 'project', 'locale')


…older to kalite/project/locale.

mrpau-eugene · 2017-11-04T04:15:25Z

@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 · 2017-11-07T07:24:28Z

Originally, the instructions are:

Remove old functionality: Stop unpacking .po files from content packs
Remove old functionality: Remove ~/.kalite/locale from LOCALE_PATHS, it shouldn’t be necessary anymore
Ensure that new path kalite/project/locale works

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:

Remove old functionality: Stop unpacking .po files from content packs
Remove old functionality: Remove ~/.kalite/locale from LOCALE_PATHS, it shouldn’t be necessary anymore
The above contains {lang}_metadata.json so we should move the {lang}_metadata.json to a new folder, maybe ~/.kalite/language-metadata/
Change update_jsi18n_file output path to somewhere inside kalite/project
Change the makemessages command output path to kalite/project/locale
For existing upgrades, copy the {lang}_metadata.json to the new metadata folder and delete the old one.
Ensure that new path kalite/project/locale works

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

benjaoming · 2017-11-07T10:44:37Z

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 · 2017-11-13T15:48:19Z

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 · 2017-11-14T05:50:06Z

@benjaoming sure! just ping me anytime :)

Stop unpacking .po files from content packs.

02e36b6

mrpau-eugene self-assigned this Oct 2, 2017

mrpau-eugene requested a review from benjaoming October 2, 2017 02:08

benjaoming reviewed Oct 4, 2017

View reviewed changes

kalite/settings/base.py Outdated

if not os.path.exists(DEFAULT_DATABASE_DIR):

os.mkdir(DEFAULT_DATABASE_DIR)

ensure_dir(USER_WRITABLE_LOCALE_DIR)

This comment was marked as spam.

Sign in to view

benjaoming suggested changes Oct 4, 2017

View reviewed changes

mrpau-eugene added 2 commits October 11, 2017 15:50

Do not use ensure_dir

6c24383

Specify missing path argument

d0ec174

benjaoming suggested changes Oct 11, 2017

View reviewed changes

mrpau-eugene added 2 commits October 13, 2017 15:25

Change metadata location to language-metadata folder and set locale f…

47adf62

…older to kalite/project/locale.

Use KALITE_LOCALE_APP_DIR

0238c81

mrpau-eugene changed the title ~~Stop unpacking .po files from content packs~~ [WIP] Stop unpacking .po files from content packs Oct 23, 2017

mrpau-eugene added 4 commits November 9, 2017 15:08

Move output path to kalite/project/locale

7b848d5

Extract metadata files to ~/.kalite/locale/{lang}

a50a5fe

Change locale directory.

447f057

Fix test for makemessages

35a4315

benjaoming mentioned this pull request Nov 13, 2017

Move UI translations from content packs to repository #5537

Closed

2 tasks

benjaoming closed this Nov 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mrpau-eugene commented Oct 2, 2017 •

edited

Loading

codecov bot commented Oct 2, 2017 •

edited

Loading

This comment was marked as spam.

benjaoming left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

benjaoming commented Oct 4, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

mrpau-eugene commented Nov 4, 2017

mrpau-eugene commented Nov 7, 2017

benjaoming commented Nov 7, 2017

benjaoming commented Nov 13, 2017

mrpau-eugene commented Nov 14, 2017

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

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

Conversation

mrpau-eugene commented Oct 2, 2017 • edited Loading

TODO

Issues addressed

codecov bot commented Oct 2, 2017 • edited Loading

Codecov Report

This comment was marked as spam.

benjaoming left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

benjaoming commented Oct 4, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

mrpau-eugene commented Nov 4, 2017

mrpau-eugene commented Nov 7, 2017

benjaoming commented Nov 7, 2017

benjaoming commented Nov 13, 2017

mrpau-eugene commented Nov 14, 2017

mrpau-eugene commented Oct 2, 2017 •

edited

Loading

codecov bot commented Oct 2, 2017 •

edited

Loading