Closed igobranco closed 1 year ago
Thanks for the pull request, @igobranco! 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.
Patch and project coverage have no change.
Comparison is base (
a02771f
) 91.86% compared to head (29bf48b
) 91.86%.:exclamation: Current head 29bf48b differs from pull request most recent head 7ce4566. Consider uploading reports for the commit 7ce4566 to get more accurate results
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Hi @asadazam93! Would you mind taking a look at this and merging if all looks good? Thanks!
@igobranco updated the branch from base via a merge commit.
Hi @asadazam93! Friendly ping on this :)
@awais-ansari I've rebased the branch!
Hi @igobranco! The tests are up and running again. All looks good, but there are branch conflicts that need to be fixed. Thanks!
@awais-ansari and @mphilbrick211 I've rebased and resolved the conflicts.
@igobranco Are all these translation languages supported by the edx-platform? And why have you added the translations text as well? I think that is expected it to be pulled in from transfix
@brian-smith-tcril can you chime in on the question above?
@asadazam93
Are all these translation languages supported by the edx-platform?
Looking at frontend-app-discussions within transifex (https://app.transifex.com/open-edx/edx-platform/frontend-app-discussions/) it seems we have |
lang code | translated | reviewed |
---|---|---|---|
pt-PT | 99.53% | 99.53% | |
uk | 98.58% | 0% | |
ru | 33% | 0% | |
hi | 30.66% | 0% | |
cs | 100% | 100% | |
es-AR | 100% | 100% | |
es-ES | 100% | 100% | |
fa-IR | 85.38% | 77.83% |
It's worth asking in the translations working group to confirm which languages can be added https://openedx.atlassian.net/wiki/spaces/COMM/pages/3157524644/Translation+Working+Group
Why have you added the translations text as well? I think that is expected it to be pulled in from transfex
My assumption is that these came in via a manual make pull_translations
run, but I would need @igobranco to confirm that. That being said, the convention is to add [lang-code].json
files that only contain {}
and let the automated jobs fill in those files (see this commit from edx-transifex-bot
as an example https://github.com/openedx/frontend-app-discussions/commit/f7ad94997d8a6f1c8c5b3cf766216bb5dd713b6a)
My assumption is that these came in via a manual
make pull_translations
run, but I would need @igobranco to confirm that. That being said, the convention is to add[lang-code].json
files that only contain{}
and let the automated jobs fill in those files (see this commit fromedx-transifex-bot
as an example f7ad949)
Yes, you are right. I didn't know that convention. I've updated the PR so the files contains only the {}
.
How the edx-transifex-bot
knows which languages to pull from transifex?
@igobranco
How the edx-transifex-bot knows which languages to pull from transifex?
it just runs make pull_translations
@igobranco Please rebase this PR and also add a proper description.
@igobranco 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
Add new languages: pt-PT, uk, ru,hi, cs, es-AR, es-ES, fa-IR