-
Notifications
You must be signed in to change notification settings - Fork 10
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
Refactor convert_jsons_to_xcstrings.py for Improved File Handling and Error Management #62
Refactor convert_jsons_to_xcstrings.py for Improved File Handling and Error Management #62
Conversation
- Refactored the code to load all language JSON files into memory once, reducing redundant file access. - Improved translation lookup efficiency by using a dictionary for language data, resulting in faster processing of keys. - Enhanced overall script performance, especially beneficial for larger datasets
- Added a check to ensure the 'jsons' folder exists, exiting with an error message if it is missing.
- Implemented a try-except block for loading 'en-US.json' to catch FileNotFoundError. - Added a user-friendly error message to indicate if the base language file is missing.
- Changed variable name from 'file' to 'base_language_data' for better readability and to clearly indicate its purpose.
Thank you for the pull request!The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and i18n rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist
|
Could you check this out for a first check as with the last one, @Jag-Marcel? :) |
@@ -10,35 +10,53 @@ | |||
import os | |||
|
|||
directory = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | |||
json_dir_list = os.listdir(os.path.join(directory, "jsons")) | |||
# Define the path to the "jsons" folder |
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.
Quick thing, @OmarAI2003: For Pythonic comments if it's its own line then there should be punctuation, so this should have a period.
|
||
translation = lang_json[key] if key in lang_json else "" | ||
for key in base_language_data: # Iterate over each key in the base language file |
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.
These commends though don't need punctuation as you have them, but then as they're inline the first letter shouldn't be capitalized. The first word of a comment is only capitalized when it's its own line.
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.
These commends though don't need punctuation as you have them, but then as they're inline the first letter shouldn't be capitalized. The first word of a comment is only capitalized when it's its own line.
I’ve made the edits to the comments as suggested. Thank you for pointing out the details; I’m learning a lot from your feedback!
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.
Very minor changes here @OmarAI2003 :) One thing to note is that we should always do inline comments two spaces after the code that they follow as this is black/ruff code standard. We don't have formatting in this repo as we want to keep it as small as we can and it's just four Python scripts. No stress :)
Thanks for your continued support for these scripts!
This PR introduces several improvements discussed here to the
convert_jsons_to_xcstrings.py
script used for converting Scribe-i18n'sLocalizable.xcstrings
file to localization JSON files.Changes Made
Folder Existence Check:
Safe File Handling:
with open(...)
) to ensure files are properly closed after being read or written, thus improving resource management.Loading Language Files:
Improved Key Handling:
dict.get()
for retrieving translations to simplify the logic and improve readability.Notes