-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: atlas push pull scripts: FC-55 #317
Conversation
Thanks for the pull request, @Amr-Nash! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
9a947c7
to
d51c1a8
Compare
@OmarIthawi could you please review this PR if we are done with the ios PR? |
Tested on both extract and pull (below):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Amr-Nash! I've tested the pull request and it worked on both combine and pull.
The README looks good and I have no comments on it.
I built the application with Android studio and it worked okay.
Please address the minor comment and it should be good to go.
@brian-smith-tcril @volodymyr-chekyrta please take a look and approve the tests to run. |
Thanks @Amr-Nash, please squash the commits to fix the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the error handling.
Done.
My pleasure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more comment please Amr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add the language renaming after split so it works with fr_CA
language for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested ✅
Works fine for me 👍
@OmarIthawi I squashed the commits. Now, it is ready to be merged. |
Extract will extract the English language resources from all modules to the I18N folder. Pull will pull all other languages translations from the openedx-traslations repository to the i18n folder then split them to thier modules.
22d5f7a
to
456cd50
Compare
@Amr-Nash 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
This pull request provides the CI scripts and developer tooling for the Atlas Translations Management Design
proposal related to OEP-58.
Testing instructions
Testing the
pull_translations
:When the above command is executed:
I18N/
directory.If the script is working as intended:
Testing the
extract_translations
:When the above command is executed:
values/strings.xml
files in all modules will be extracted"Module_name.OLD_KEY_of_the_entry"
.I18N/src/main/res/values/strings.xml
If this script is working as intended:
I18N/src/main/res/values/strings.xml
with the corresponding comment, if present, placed before it.Q and A
1. When are source strings extracted? what do the files look like?
I18N/src/main/res/values/strings.xml
and it will look like this:2. Does anything happen to those files before they are added to openedx-translations
The only extracted file is the English sources from the modules in the Android repo as it is the only translation that lives there.
3. When translation files are pulled via atlas what do the files look like?
When the translation files are pulled via atlas the files' structure is going to look like the structure below:
As an example the values-es/strings.xml would look like:
I18N/src/main/res/values-es/strings.xml
4) What happens to those files after
atlas pull
to make them work in the app build process?After
atlas pull
is completed, all language translation files are retrieved, similar to the process described in the previous question. Subsequently, a Python script is executed to process each file. This script iterates through each translation file, splits it, removes the module name from the keys, and organizes each entry into its corresponding module.For instance, the
I18N/src/main/res/values-es/strings.xml
file is split into two separate files:Module_one/src/main/res/values-es/strings.xml
Module_two/src/main/res/values-es/strings.xml
Inside
Module_one/src/main/res/values-es/strings.xml
, the content would resemble:Similarly, inside
Module_two/src/main/res/values-es/strings.xml
, it would appear as:Once the Python script completes its task and the files are split, the make script removes the entire
I18N
directory and its contents.5) Why is this needed?
This new process is being introduced to have the best combination of Developer Experience and Translator Experience.
The best experience for Translators requires combining source strings into as few transifex resources as possible. The best experience for Engineers requires splitting translation source files to fit within the modular architecture.
6) What should live in openedx translations?
Translations that were done by the open-edx translators are going to be stored in openedx translations.
7) What should live in atlas?
Atlas is the tool that we are using to pull the translations from openedx translations; therefor, no translations are stored in atlas.
8) What should live in the openedx-app-Android repo?
In the Android app repo only the English sources are going to be stored.
TODO
make extract_translations