Skip to content

Conversation

@ovnicraft
Copy link
Contributor

Added more strings to be translated.

  • Level education
  • Checklist default values

@ovnicraft
Copy link
Contributor Author

@singingwolfboy or @cahrens can you review it please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLs can't be translated

@sarina
Copy link
Contributor

sarina commented Dec 12, 2013

👍

@ovnicraft
Copy link
Contributor Author

I need to sign CA pdf ? @sarina

@singingwolfboy
Copy link
Contributor

@ovnicraft Yes, you will need to sign the contributor agreement and add yourself to the AUTHORS file before we can merge your pull request.

@sarina
Copy link
Contributor

sarina commented Dec 12, 2013

@ovnicraft see #1923 for better instructions on how to do so, and let us know when you've mailed in the agreement & updated AUTHORS

@ovnicraft
Copy link
Contributor Author

@sarina I sent the email and added myself to AUTHORS

@sarina
Copy link
Contributor

sarina commented Dec 16, 2013

@ovnicraft I checked and we haven't received your agreement. Did you sign under a name other than "Cristian Salamea"? Do you think you can re-send it to the email address listed on the agreement?

@ovnicraft
Copy link
Contributor Author

@sarina I resent the email i sent to the email in the CA.

@sarina
Copy link
Contributor

sarina commented Dec 16, 2013

@ovnicraft we're still not receiving it. I'm going to email you directly to the address you put in AUTHORS - please reply and attach your agreement to that. Sorry for the hassle 😢

@sarina
Copy link
Contributor

sarina commented Dec 16, 2013

@ovnicraft Ah! Never mind! We found it. Please follow up with Jennifer's email, thanks!

@ovnicraft
Copy link
Contributor Author

@sarina i push a new commit can you review it ?

@sarina
Copy link
Contributor

sarina commented Dec 17, 2013

@adampalay not sure if you got notified from my above comment - can you check the content in instructor_dashboard.html? I'm pretty sure we can't translate those strings because the strings themselves provide the button action.

@adampalay
Copy link
Contributor

@sarina and @ovnicraft , yeah, unfortunately we can't translate the buttons on instructor_dashboard.html because the strings provide the button action.

@sarina
Copy link
Contributor

sarina commented Dec 17, 2013

Thanks. @ovnicraft can you just revert that commit?

@sarina sarina closed this Dec 17, 2013
@sarina sarina reopened this Dec 17, 2013
@ovnicraft
Copy link
Contributor Author

@sarina its done !

@sarina
Copy link
Contributor

sarina commented Dec 18, 2013

@ovnicraft your branch needs a rebase, as there are some conflicts. See https://github.com/edx/edx-platform/blob/master/CONTRIBUTING.md for some help with rebasing.

@singingwolfboy can you give a thumbs up on this?

@ovnicraft
Copy link
Contributor Author

@sarina what conflicts ?

@ovnicraft
Copy link
Contributor Author

@sarina i read CONTRIBUTING.md so i need to squash commits ?

Confirm me please.

@sarina
Copy link
Contributor

sarina commented Dec 18, 2013

I don't know what the conflicts are, simply that they exist, because I can't merge the pull request. You can't see this but github tells me: "We can’t automatically merge this pull request." which means there are conflicts.

You don't need to squash commits, just rebase your branch onto master. I do this by git fetch; git rebase origin/master - I'm not exactly sure how to do this onto a fork. but basically this replays your commit(s) on top of the latest version of master, and allows you to fix conflicts.

@ovnicraft
Copy link
Contributor Author

So i rebase it, conflicts was in AUTHORS, i solved.

@sarina
Copy link
Contributor

sarina commented Dec 18, 2013

Thanks! As soon as @singingwolfboy gives a thumbs-up we will merge.

@singingwolfboy
Copy link
Contributor

👍 ✨ ❤️ 👍 :shipit: 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah shoot @singingwolfboy we missed this. You can't translate this file yet - we can't have django dependencies in xmodule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run the CMS after doing the dummy translations and it broke. @ovnicraft try

rake i18n:extract; rake i18n:dummy; rake i18n:generate; rake cms

the rake cms call will give an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my post here: https://groups.google.com/forum/#!topic/edx-code/nKHfn9e4Fc4 for the particular error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarina So course_module.py can be translated i change it this, but there is any way thought to solve this ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now we do not have a solid way of translating non-django components of edx-platform. When we do, we will document how it must be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok i will change it and push.

@nedbat
Copy link
Contributor

nedbat commented Dec 31, 2013

Hi everyone, sorry to chime in late on this. The bad news is: all of the _() function calls on the help and description strings need to be done differently. Open edX needs to be internationalized so that the user can select their language. This means that looking up localized strings can only happen in the context of a current user. The help strings (for example) make the _() function call when the module is imported, which is too early.

I'll write an explanation of how to achieve this, and put a link here....

@sarina
Copy link
Contributor

sarina commented Jan 3, 2014

@nedbat that makes sense. Will the solution be coming soon?

@ovnicraft
Copy link
Contributor Author

@nedbat i have some experience with internationalization and i consider if edX needs to give to user the change to select their language (Ex: change it from his preferences) could reload modules BAD choice IMO and another option is store language texts in DB and read it in each request translating from user preferences.

So waiting for your link.

Thanks !

PD: will update the PR tonite i a coming from vacations.

@nedbat
Copy link
Contributor

nedbat commented Jan 3, 2014

@ovnicraft Django provides all the support we need to do per-user localization, we just need to mark the strings properly. We're working out what is the right technique.

@nedbat
Copy link
Contributor

nedbat commented Jan 22, 2014

@ovnicraft Sorry for how long this has taken. We've been working hard on the i18n work for the LMS. We now have updated docs at https://github.com/edx/edx-platform/blob/master/docs/en_us/developers/source/i18n.rst that include some details of how to do these lazy translations.

Another factor that makes me want to hold off on merging this: these strings are only visible in Studio, and we'd like to focus our translators' efforts on translating LMS strings. We are working on a way to separate the LMS and Studio strings, but these strings are not in the cms/* tree, so it will be difficult to separate them out. If we could wait a little bit more to get these strings marked, it would help us stay focused on the LMS translation. I hope that isn't too inconvenient. Thanks.

@ovnicraft
Copy link
Contributor Author

@singingwolfboy thanks i am reading that (sent before) to know well about translation, by now close the PR.

@ovnicraft ovnicraft closed this Jan 22, 2014
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Jun 23, 2017
* Fix review, add bok-choy.  openedx#1867

* Fix bug for login page message. openedx#1932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants