Closed OmarIthawi closed 3 months ago
Thanks for the pull request, @OmarIthawi! 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.
@miankhalid @volodymyr-chekyrta I've assigned you as reviewers, but please feel free to re-assign to someone else as you see fit.
@volodymyr-chekyrta @brian-smith-tcril I've updated the proposal with the prefix option, and addressed all the other comments.
Please let me know if there's anything else.
Thanks a lot @brian-smith-tcril and @volodymyr-chekyrta for the reviewing this so quick :)
I'm happy about the outcome, and we'll start working on it as soon as possible.
I've removed the Makefile
since it was added without scripts so this pull request becomes mergeable:
For reference, here was the Makefile contents:
openedx-app-ios $ cat Makefile
clean_translations_temp_directory:
rm -rf I18N/I18N/
pull_translations:
make clean_translations_temp_directory
atlas pull $(ATLAS_OPTIONS) translations/openedx-app-ios/I18N/I18N/:I18N/I18N/
python scripts/split_translation_files.py
make clean_translations_temp_directory
combine_translations:
make clean_translations_temp_directory
python scripts/combine_translations_files.py
We'll add it back once we implement the referenced scripts.
@OmarIthawi, any concerns about switching the PR target branch to the develop?
We follow feature -> develop -> main
git flow
@brian-smith-tcril I've added a detailed step-by-step workflow and a new notable changes
section.
Please double check and let me know if you have any notes.
@brian-smith-tcril all minor changes are applied. Good catches I should say :)
I think docs proof-reading has a legit AI use case the GitHub needs! I miss such feature.
@brian-smith-tcril all minor changes are applied. Good catches I should say :)
I think docs proof-reading has a legit AI use case the GitHub needs! I miss such feature.
I've used the free version of grammarly when writing OEPs/ADRs before and it's been helpful
Thanks for putting up with all the back and forth during the review process! I left one tiny last comment but this looks good to go. Approved :+1:
I've used the free version of grammarly when writing OEPs/ADRs before and it's been helpful
I'll try that! Writing RST requires a lot of effort which definitly need some tooling help esp. an ESL person.
Thanks for putting up with all the back and forth during the review process! I left one tiny last comment but this looks good to go. Approved 👍
Thanks for all the reviews! It's sometimes hard to articulate ideas and follow up with edit notes, but it makes it much clearer for future reviewers.
I'll try that! Writing RST requires a lot of effort which definitly need some tooling help esp. an ESL person.
For sure! It's hard enough for me with English as a first language!
Thanks @volodymyr-chekyrta and @brian-smith-tcril for quickly triaging this decision. Both of you have already approved it. Kindly merge.
We'll start working in the coming few days.
Please review this proposal. We're set to start on April 1st, 2024.
Please let us know if you have any questions or objections.
I've omitted many OEP-58 related details, to keep the discussion focused on the mobile parts.
Preview link the proposal doc:
This pull request is part of the FC-0055 project which implements the Translation Infrastructure update OEP-58 for mobile apps.