-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 pull for studio-frontend translations | FC-0012 #34261
Conversation
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. |
0aaa342
to
47816d6
Compare
47816d6
to
c494604
Compare
f9da08a
to
a7f81d0
Compare
a7f81d0
to
01d4f19
Compare
@timmc-edx this is one last piece to add |
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.
Looks good!
I got an error when I ran the command EDX_PLATFORM_SETTINGS=production LMS_CFG=lms/envs/minimal.yml STUDIO_CFG=lms/envs/minimal.yml OPENEDX_ATLAS_PULL=true make pull_translations
that we're using in deployments, but it looks like that's just because this branch doesn't have the change to minimal.yml from #34285. When I merged the two together locally, it worked fine. 👍
01d4f19
to
930df16
Compare
Thanks @timmc-edx. I've rebased the branch so that should fix the issue. @timmc-edx When do you plan to use cc: @feanil @brian-smith-tcril for your review. |
We started deploying LMS and Studio with atlas translations on Monday morning or so, so 2U is ready for this to merge whenever. |
@OmarIthawi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
This pull request pulls translations via atlas for `studio-frontend` and refactor `load_sfe_i18n_messages` to load new translations into `conf/plugins-locale/studio-frontend` instead of relying on deprecated node.js package bundled translations.
That's great!! We're off for a great start :) |
Change
This pull request pulls translations via atlas for
studio-frontend
and refactorload_sfe_i18n_messages
to load new translations intoconf/plugins-locale/studio-frontend
instead of relying on deprecated node.js package bundled translations.Background
Studio Frontend is a Micro-frontend-like repository bundled in the the
edx-platform
.Currently the edx-platform loads translations for
studio-frontend
from thenode_modules
package which assumes a pre-OEP-58 setup.Breaking change
After merging this pull request, studio will require running either
make OPENEDX_ATLAS_PULL=yes pull_translations
ormake pull_studio_frontend_translations
during deployment or build to pull translations. Otherwise, it'll appear in English only.Merge this change once 2U is aware of the change and accepts it.
We can make it backward-compatible, but the FC-0012 is already being closed so unless 2U object, we'll ship a breaking change.
Testing
devstack logs
devstack screenshots
Original npm-package bundled translations
Using the
edx-platform/conf/plugins-locale/studio-frontend
empty directoryUsing the
edx-platform/conf/plugins-locale/studio-frontend
pulledDeadline
As soon as possible to fix minor remaining items of FC-0012 project.
Related
This pull request is part of the FC-0012 project which implements the Translation Infrastructure update OEP-58.