-
Notifications
You must be signed in to change notification settings - Fork 15
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
chore: import translations from openedx-translations #45
Conversation
Thanks for the pull request, @Ian2012! 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. |
@OmarIthawi how can I compile the |
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.
Thanks @Ian2012!
@Ian2012 run |
73eee21
to
18dea33
Compare
@OmarIthawi ready, but translations will not work till the I will wait till that PR is merged and the translation file updated to do this contribution |
@Ian2012 how about
you could also compile via Regardless of the method, this pull request needs |
@OmarIthawi The |
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.
@OmarIthawi The
.mo
are in the PR but the translations are not working correctly as the templates are not using thetranslate
template tag
You're right! I'm sorry I missed that! It looks great now. Please reset the tests because codecov has failed.
18dea33
to
8c3c554
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #45 +/- ##
=======================================
Coverage 92.41% 92.41%
=======================================
Files 5 5
Lines 224 224
=======================================
Hits 207 207
Misses 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d381207
to
6a9a9f3
Compare
@OmarIthawi tests are successful now |
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.
One required change and it should be ready to go. Thanks @Ian2012
Please let me know when it's ready for review again.
feedback/locale
Outdated
@@ -0,0 +1 @@ | |||
feedback/locale |
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.
I don't think this link is needed. Please remove it.
We need the following link though:
cd feedback && ln -sd conf/locale translations
Similar to this one: https://github.com/openedx/xblock-drag-and-drop-v2/blob/master/drag_and_drop_v2/translations
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.
@OmarIthawi I've created the new simlink, rebase with master
changes and updated the translations files, however, translations are still not working.
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.
The symlink path needs to be updated. please run the following commands to create the right symlink:
cd FeedbackXBlock # repo root
cd feedback # package root
rm conf/locale/locale
ln -sd conf/locale translations
This is the only way translations can work.
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.
I've ran the commands, and pushed the changes, still not working
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.
I don't see the link in the commits. please try again.
Try squashing all the changes and push force.
I see the add6ce5 commit adds the link, but it doesn't appear of the Files tab of the PR https://github.com/openedx/FeedbackXBlock/pull/45/files
Something is wrong in the git commit history
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.
I've squashed the commits and still not working. The translations simlink exists in the repository: https://github.com/openedx/FeedbackXBlock/blob/master/feedback/translations
add6ce5
to
cffac36
Compare
cffac36
to
91d57a4
Compare
91d57a4
to
b10be52
Compare
Hi @feanil - just checking to see if this can be merged? |
Tagging @OmarIthawi and asking him to re-review since he was looking at this and is more familiar with the translations work. |
@Ian2012 I'm sorry to let you know that we'll have to close this pull request. As of Redwood, we no longer commit Please consider checking OEP-58 for more information about the new translation workflow. |
@Ian2012 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. |
Description
We are manually importing translation files from openedx-translations while openedx-atlas is in place.