Skip to content

i18n fix - #9868

Merged
nedbat merged 1 commit into
openedx:masterfrom
louyihua:i18n_fix
Sep 24, 2015
Merged

i18n fix#9868
nedbat merged 1 commit into
openedx:masterfrom
louyihua:i18n_fix

Conversation

@louyihua

Copy link
Copy Markdown
Contributor
  1. Add missing '_()' in capa_base.py & video_xfields.py
  2. Fix a bug in instructor_dashboard.py that causes crash when ANALYTICS_DASHBOARD_NAME contains non-ascii characters.
  3. Workaround for mis-extractions inside .underscore files when using gettext along with interpolate.

@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @louyihua! I've created OSPR-823 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email 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 still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U needs triage labels Sep 22, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should change these two messages to use %(name)s format also.

@louyihua
louyihua force-pushed the i18n_fix branch 3 times, most recently from 367e9c1 to c849bdb Compare September 23, 2015 05:23
@louyihua

Copy link
Copy Markdown
Contributor Author

@nedbat This is fixed.

@nedbat

nedbat commented Sep 23, 2015

Copy link
Copy Markdown
Contributor

👍

@sarina

sarina commented Sep 24, 2015

Copy link
Copy Markdown
Contributor

@louyihua Thanks for this fix. Would you mind also updating our i18n guidelines in the docs repo to explicitly call out that we can't have gettext and interpolate on the same line? http://edx.readthedocs.org/projects/edx-developer-guide/en/latest/internationalization/i18n.html#underscore-template-files

Feel free to tag @nedbat in that pull request ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since you're here, can you please change this to use a better variable, eg %(start_date)s?

@sarina

sarina commented Sep 24, 2015

Copy link
Copy Markdown
Contributor

@louyihua 👍 once you fix that nit about the variable name.

@nedbat can you take care of merging this when that's done?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dsego gettext doesn't work in the same line as interpolate in underscore templates. Please be sure that you're properly testing your features for i18n. Further, please make sure to give formatting variables proper names when used in translator comments.

1. Add missing '_()' in capa_base.py & video_xfields.py
2. Fix a bug in instructor_dashboard.py that causes crash when ANALYTICS_DASHBOARD_NAME contains non-ascii characters.
3. Workaround for mis-extractions inside .underscore files when using gettext along with interpolate.
@louyihua

Copy link
Copy Markdown
Contributor Author

@sarina @nedbat OK, the variable name is fixed.

nedbat added a commit that referenced this pull request Sep 24, 2015
@nedbat
nedbat merged commit 45176c7 into openedx:master Sep 24, 2015
This was referenced Sep 29, 2015
clintonb pushed a commit that referenced this pull request Sep 29, 2015
Variable name was broken by a commit in #9868.

ECOM-2431
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants