openedx-webhooks
commented
1 year ago Thanks for the pull request, @shadinaif! 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:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
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.
codecov[bot]
shadinaif
OmarIthawi
feat: add two new options,
merge-po-filesandno-segmentAfter we passed a lot of steps in FC-0012 project; we see a need to add the following options to
extractcommand because it'll serve us in a shared logic between many other packages (especially XBlocks). So, instead of repeating the same logic in Makefiles; we just add the options. DRY, and more readable--no-segment: we have a logic inextractcommand that generates (partial files). This is needed because of the complicity ofedx-platform. But it creates a burden on small XBlocks that are always interested in the finaldjango.po. Before this option; we have to always rename the generateddjango-partial.potodjango.po. The option will also skip the segment process is any--merge-po-files: small packages (like small XBlocks) don't need to have two separate translation files for HTML and JS. We usually merge the generated filedjangojs.pointodjango.pomanually. This option will take care of that without repeating the steps in all small packages[x] Related unit tests added
[x] All tests pass successfully
[x] Testing behavior in another package (see Local Manual Testing section)
Local Manual Testing
I used xblock-drag-and-drop-v2 locally to test
extractcommand:Set this PR in requirement instead of the current
1.1.0version. Done intest.infileedx-i18n-tools # For i18n_tool dummy
git+https://github.com/Zeit-Labs/i18n-tools.git@b739443cfcfe411c0dfedcc94b245585f1350264#egg=edx-i18n-tools==1.1.0
Run
make upgradeRemove
django.pofile andtext.posymbolic link fromxblock-drag-and-drop-v2/drag_and_drop_v2/conf/locale/en/LC_MESSAGESUpdate
extract_translationscommand inMakefilecd $(WORKING_DIR) && i18n_tool extract
mv $(EXTRACTED_DJANGO_PARTIAL) $(EXTRACTED_DJANGO)
Safely concatenate djangojs if it exists. The file will exist in this repo, but we're trying to follow a pattern
between all repositories that use i18n_tool
if test -f $(EXTRACTED_DJANGOJS_PARTIAL); then \
msgcat $(EXTRACTED_DJANGO) $(EXTRACTED_DJANGOJS_PARTIAL) -o $(EXTRACTED_DJANGO) && \
rm $(EXTRACTED_DJANGOJS_PARTIAL); \
fi
cd $(WORKING_DIR) && i18n_tool extract --merge-po-files --no-segment
mv $(EXTRACT_DIR)/django.po $(EXTRACT_DIR)/text.po
Run
make extract_translations.text.pois generated successfullyCommit changes (because there are tests that validate translation to ensure all new are committed)
Run
py38-django32andtranslations-django32tests of xblock-drag-and-drop-v2 package. All passed