-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Deploy all used locales #28922
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
Deploy all used locales #28922
Conversation
Hi @AntonEvers. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. |
@magento run all tests |
@magento run Unit Tests |
@magento run all tests |
Hi @AntonEvers. Thanks for collaboration. This PR was reviewed on Community Contribution Triage meeting. The recording of the meeting you can look by the link |
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.
@AntonEvers thank you so much for your contribution!
This is really important improvement!
e2b1fcb
to
8d2376b
Compare
@ihor-sviziev please let me know if this needs some cleaning up / relocation of logic. I can imagine that the coupling of objects is getting larger than desirable at the moment. |
@magento run all tests |
I went ahead and cleaned up a little. Please let me know if you see any issues with this. |
@magento run Static Tests |
@AntonEvers from first look - looks good, but let’s wait for test results. Also would be good to cover these changes with some automated tests, I think we need to have integration test there, similar to this https://github.com/magento/magento2/blob/40a787632f36ca67f8fff8c451e1e8d1bdbaf9f6/dev/tests/integration/testsuite/Magento/Setup/Console/Command/DependenciesShowFrameworkCommandTest.php will you be able to cover your change with such test? @magento run all tests |
@magento run Static Tests |
@magento run Static Tests |
`bin/magento setup:static-content:deploy --language=all` currently only deploys en_US, instead of the requested "all". In the process repair the language argument in the deployment command. Remove DB dependency and deploy theme specific languages Add deployment config for Admin locale generation Reduce object coupling by introducing a new class Fix code sniffer errors add unit tests Suppress PHPMD.CouplingBetweenObjects resolve static test issues and Semantic Version Checker issue resolve static test results
@magento run all tests |
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.
Good job! Thank you so much for this improvement!
Hi @ihor-sviziev, thank you for the review. |
@magento create issue |
✔️ QA Passed Manual testing scenario:
Before: ✖️ only en_US are deployed; After: ✔️ All configured languages are deployed
After: ✔️ All configured languages except English are deployed
After: ✔️ Only en_US is deployed
After: ✔️ Only en_US is deployed
After: ✔️ Only de_DE is deployed as the languages argument precedes over the language option |
Hi @AntonEvers, thank you for your contribution! |
Description (*)
bin/magento setup:static-content:deploy --language=all
currently onlydeploys en_US, instead of the requested "all". This pull request aims to deploy all used languages for the frontend and all configured languages by admin users, when no language parameter is given. en_US is always additionally deployed by default.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
bin/magento setup:static-content:deploy --language=all
and verify that all configured languages, plus en_US are deployed.bin/magento setup:static-content:deploy --language=all --exclude-language=en_US
and verify that en_US is skipped.bin/magento setup:static-content:deploy en_US
and verify that only en_US is deployedbin/magento setup:static-content:deploy --language=en_US
and verify that only en_US is deployedbin/magento setup:static-content:deploy -l en_US
and verify that only en_US is deployedbin/magento setup:static-content:deploy --language=en_US en_UK
and verify that only en_UK is deployed as the languages argument precedes over the language optionQuestions or comments
Contribution checklist (*)
Resolved issues: