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

Review the i18n tooling to ensure the deletion of the old CSV files with context #11861

Closed
radinamatic opened this issue Feb 12, 2024 · 11 comments
Assignees
Labels
DEV: tools Internal tooling for development P0 - critical Priority: Release blocker or regression TAG: dev experience Build performance, linting, debugging...

Comments

@radinamatic
Copy link
Member

Observed behavior

During the most recent update of the translated strings from Crowdin, I run into a rabbit-hole where no mater what I did, the translations downloaded from Crowdin would be older (very old) versions, and not those approved in the most recent string freeze. @rtibbles suggested I check my local repo for the old CSV context files that have not been cleaned up properly, and that indeed was the case.

1 2
2024-02-12_22-04-08 2024-02-12_22-07-54

As soon as I deleted the old CSV files (some 2+ years old) that were not part of the most recent execution of the make i18n-download... command, proper translations were being fetched again.

Errors and logs

Expected behavior

make i18n-download... command should fetch the most recently approved translations for the exact branch it's being invoked.

User-facing consequences

Older and incorrect translations can sneak back in the current UI.

Steps to reproduce

Check if you have old CSV context files in the locale/.../LC_MESSAGES folders, and you might e able to reproduce.

Context

  • Kolibri version: 0.16b13
  • Operating system: Ubuntu 22.04
@radinamatic radinamatic added P0 - critical Priority: Release blocker or regression TAG: dev experience Build performance, linting, debugging... DEV: tools Internal tooling for development labels Feb 12, 2024
@oge1ata
Copy link
Contributor

oge1ata commented Mar 25, 2024

Hi I would love to take a look at this issue as a GSOC applicant if possible.

@rtibbles
Copy link
Member

Hi @oge1ata - absolutely, that would be great. This is the function that is currently downloading the CSV files - I think the ideal fix would clear out each language folder before writing the files from the zip into it.

@oge1ata
Copy link
Contributor

oge1ata commented Mar 25, 2024

Thank you @rtibbles - I've looked at it. What zip file exactly would I be working with? And would this mean I would have to clear out the translated files for every language and redo them? Apologies if the questions are a lot

@rtibbles
Copy link
Member

The zip file gets downloaded from crowdin - unfortunately, that requires access to our translations, which I don't know that I can share.

I also realize I failed to actually share the link to the function! https://github.com/learningequality/kolibri/blob/develop/packages/kolibri-tools/lib/i18n/crowdin.py#L339

I suspect just emptying out the locale_dir_path before moving all the files from the zip in would be sufficient: https://github.com/learningequality/kolibri/blob/develop/packages/kolibri-tools/lib/i18n/crowdin.py#L393

@oge1ata
Copy link
Contributor

oge1ata commented Mar 25, 2024

Thank you again @rtibbles I checked how to reproduce from the steps, but going through the language LC_MESSAGES, I don't have old csv files, would there be another way to reproduce it? So that I could test it?

I also followed your recommendation in emptying out the locale_dir_path before moving all files to the zip, now it's just to test it out... I believe

Edit: I'm checking out some things now and following the documentation, so that I can check it out

@oge1ata
Copy link
Contributor

oge1ata commented Mar 26, 2024

Hey @rtibbles. I've made some changes too the crowdin.py file, should I make a pull request or you see it first?

@rtibbles
Copy link
Member

Please make the pull request, that's the easiest place for me to review. If you don't think it's ready to go, feel free to open it as a draft pull request!

@oge1ata
Copy link
Contributor

oge1ata commented Mar 26, 2024

I've done so #12020

@rtibbles
Copy link
Member

Thanks, I added a review!

@oge1ata
Copy link
Contributor

oge1ata commented Mar 27, 2024

Oh yes I saw, I used glob this time for the removal taking in your review

@rtibbles
Copy link
Member

This has been fixed by #12020 - old CSV files in language directories will be cleared out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: tools Internal tooling for development P0 - critical Priority: Release blocker or regression TAG: dev experience Build performance, linting, debugging...
Projects
None yet
Development

No branches or pull requests

3 participants