Skip to content
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: add translations for Persian language #271

Conversation

FatemeKhodayari
Copy link
Contributor

Translations for Persian (fa-ir language code) are added so the learner dashboard MFE will be usable for Persian users.

Closes #270

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 13, 2024
@openedx-webhooks
Copy link

Thanks for the pull request, @FatemeKhodayari! 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.

@FatemeKhodayari FatemeKhodayari force-pushed the FatemeKhodayari/add-persian-language-to-master branch from 5c114a8 to c55e1a9 Compare January 13, 2024 13:07
@itsjeyd
Copy link

itsjeyd commented Jan 18, 2024

Hi @FatemeKhodayari! Thank you for your contribution! I'm lining the changes up for a test run.

@itsjeyd itsjeyd added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jan 18, 2024
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ad56b36) 96.04% compared to head (c55e1a9) 96.36%.
Report is 1 commits behind head on master.

❗ Current head c55e1a9 differs from pull request most recent head 279f0aa. Consider uploading reports for the commit 279f0aa to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #271      +/-   ##
==========================================
+ Coverage   96.04%   96.36%   +0.32%     
==========================================
  Files         194      194              
  Lines        1844     1843       -1     
  Branches      322      326       +4     
==========================================
+ Hits         1771     1776       +5     
+ Misses         68       62       -6     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mphilbrick211 mphilbrick211 removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jan 19, 2024
@itsjeyd
Copy link

itsjeyd commented Jan 25, 2024

@FatemeKhodayari Looks like we've got a green build! I'm marking this PR as ready for review, but let me know if you were planning on making further changes to it.

@itsjeyd itsjeyd added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Jan 25, 2024
@FatemeKhodayari
Copy link
Contributor Author

FatemeKhodayari commented Jan 28, 2024

@FatemeKhodayari Looks like we've got a green build! I'm marking this PR as ready for review, but let me know if you were planning on making further changes to it.

Thanks for the checks and feedback. Although the edx-platform project on transifex is stopped and edx-translations does not have Persian yet, I can check transifex for possible updates if you think it's needed. But aside from that, don't think I have anything to add for now. Shall I sync everything with transifex or is it fine the way it is?

@itsjeyd
Copy link

itsjeyd commented Feb 2, 2024

Thanks for the details @FatemeKhodayari. I don't have enough context to answer your question, which means that now would definitely be a good time to get engineering review started :)

@leangseu-edx Would you be able to take a look at this PR?

@itsjeyd
Copy link

itsjeyd commented Feb 8, 2024

Hi @viktorrusakov, I see that you're listed as a core contributor for all frontend-* repos. Would you be able to help out here and review this PR?

For context, one of the decisions made at the first meeting of the new Maintenance WG last week was that we can let CCs take on Reviews and Merges that they can manage. That will hopefully help us get reviews for community contributions more quickly, and is why I'm pinging you here :)

CC @FatemeKhodayari

@viktorrusakov
Copy link

HI @itsjeyd! You can absolutely tag me on PRs that need help with reviews, but I'm not sure I'll be able to help much with this one as I don't know what is the current process for adding a new language to MFEs.

The code changes look good to me and I see that these translations are present in the Transifex resourse for this MFE, which means they will get updated automatically. I think this should be enough for this PR to get merged but I also know that there is an ongoing (maybe even finished already?) work on changing translations pipelines in repositories, so maybe there are additional steps we need to do which I'm not aware of.

@brian-smith-tcril could you help with this PR? I think you were heavily involved in changing translations' workflows project. Just want to make sure this is OK to merge.

@FatemeKhodayari FatemeKhodayari requested a review from a team as a code owner February 8, 2024 22:40
@FatemeKhodayari
Copy link
Contributor Author

Hi guys. I just merged master into my branch to keep it updated but then I realised that the approved workflows have gone back to "awaiting approval" state. Hope I haven't disrupted the process. Generally, what is the openedx policy for this situation? Should the developer keep the branch updated with master at all stages before merge?

@itsjeyd @viktorrusakov @mphilbrick211

@jsnwesson
Copy link
Contributor

Hey all! I'm responding on behalf of the Aperture team just to say that I've got eyes on this as well, but since our team is no longer responsible for translations I can't confidently speak on what the new translation system requires for new languages. What I can do is at least make this PR known in the openedx #wg-translations channel, and hopefully we can get this merged sooner than later!

@OmarIthawi
Copy link
Member

@FatemeKhodayari Thanks for your contribution.

Unfortunately, at the moment we don't support this workflow anymore and we're removing all the languages from Micro-frontends. Please see the related removal pull request:

Since you've already merged that in your master I suggest keeping those change in your fork until Redwood in which a new approach will be used:

You're welcome to start testing the experimantal atlas pull in Tutor MFE before it's merged and it will provide you with the latest translations on every build without needing to open pull requests like this.

Please take a look at the OEP-58 proposal which has been implemented and scheduled to be released in Redwood.

I recommend closing this pull request.

@FatemeKhodayari
Copy link
Contributor Author

@FatemeKhodayari Thanks for your contribution.

Unfortunately, at the moment we don't support this workflow anymore and we're removing all the languages from Micro-frontends. Please see the related removal pull request:

Hi Omar. Thanks for the feedback. Merging and getting all translations from a single source sounds like a good idea to me. Thanks for letting me know.

Since you've already merged that in your master I suggest keeping those change in your fork until Redwood in which a new approach will be used:

You're welcome to start testing the experimantal atlas pull in Tutor MFE before it's merged and it will provide you with the latest translations on every build without needing to open pull requests like this.

Actually I haven't merged these changes into the master branch of my fork, I have them in a separate branch. I merged the remote master into this branch to keep it updated but that doesn't matter now :))

About atlas pull, you mean the command in the # Experimental: OEP-58 Pulls translations using atlas section of Makefile? Suppose this change won't affect customized translations applied by Tutor, right?

@OmarIthawi
Copy link
Member

About atlas pull, you mean the command in the # Experimental: OEP-58 Pulls translations using atlas section of Makefile? Suppose this change won't affect customized translations applied by Tutor, right?

Yes @FatemeKhodayari. Running make OPENEDX_ATLAS_PULL=yes pull_translations should provide your Micro-frontend with the latest translations.

This is a Redwood feature which is being merged as we speak:

@FatemeKhodayari
Copy link
Contributor Author

FatemeKhodayari commented Feb 13, 2024

Yes @FatemeKhodayari. Running make OPENEDX_ATLAS_PULL=yes pull_translations should provide your Micro-frontend with the latest translations.

So, I guess from now on, only translations from openedx-translations project of transifex will appear when you pull? What about languages that aren't present in this project? For example, Persian existed in the edx-platform project of transifex but not in the openedx-translations project.

Someone requested Persian (fa-ir) some time ago in the openedx-translations project in transifex but it still isn't available for translation.

@OmarIthawi
Copy link
Member

So, I guess from now on, only translations from openedx-translations project of transifex will appear when you pull? What about languages that aren't present in this project? For example, Persian existed in the edx-platform project of transifex but not in the openedx-translations project.

Yes.

Someone requested Persian (fa-ir) some time ago in the openedx-translations project in transifex but it still isn't available for translation.

Please ping @ehuthmacher about the list of the languages we'll be adding. I believe the plan is to add about 100 more language including Persian.

cc: @shadinaif

@itsjeyd
Copy link

itsjeyd commented Feb 16, 2024

@FatemeKhodayari Just checking in about next steps. Are you planning on closing this PR as @OmarIthawi suggested above?

@itsjeyd itsjeyd added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Feb 16, 2024
@FatemeKhodayari
Copy link
Contributor Author

@FatemeKhodayari Just checking in about next steps. Are you planning on closing this PR as @OmarIthawi suggested above?

Yes sure. Hope Persian is added to the openedx-translations of transifex soon :)

@openedx-webhooks
Copy link

@FatemeKhodayari Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@FatemeKhodayari FatemeKhodayari deleted the FatemeKhodayari/add-persian-language-to-master branch February 17, 2024 04:36
@ehuthmacher
Copy link

@FatemeKhodayari, yes, we will add Persian to the openedx-translations in TX this week.

@FatemeKhodayari
Copy link
Contributor Author

FatemeKhodayari commented Feb 21, 2024

@FatemeKhodayari, yes, we will add Persian to the openedx-translations in TX this week.

Hello @ehuthmacher! That's great news! Thanks a lot :)
Hope both Persian and Persian (Iran) are added or they're merged in some way.

@itsjeyd itsjeyd removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

No translations for Persian language
8 participants