Skip to content

Sarina/more ins dash - #1195

Merged
sarina merged 2 commits into
masterfrom
sarina/more-ins-dash
Oct 3, 2013
Merged

Sarina/more ins dash#1195
sarina merged 2 commits into
masterfrom
sarina/more-ins-dash

Conversation

@sarina

@sarina sarina commented Oct 1, 2013

Copy link
Copy Markdown
Contributor

A few things I forgot in my last PR.

  • Internationalization
  • Completely remove unused enrollment code from the Student Admin page (template & coffeescript) - we decided to only expose enroll/unenroll feature on the Membership page, because that's where we can provide email notification

Review: @adampalay @flowerhack

@singingwolfboy can you very quickly verify I did the i18n here correctly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Outdated comment - my last PR made it so we now do get notifications of success events.

@sarina

sarina commented Oct 2, 2013

Copy link
Copy Markdown
Contributor Author

Thanks for the comments. I think I've addressed everything that's been pointed out.

Underscore is already loaded & there's actually stuff at the bottom of the coffee file to enforce that it is. So, sweet.

@adampalay

Copy link
Copy Markdown
Contributor

🐶 🐱 🌵 🐳 🐙 🎍 💳 💸 🎩

@flowerhack

Copy link
Copy Markdown
Contributor

All looks good to me!

@marcotuts

Copy link
Copy Markdown
Contributor

Should this get a changelog entry?

@sarina

sarina commented Oct 2, 2013

Copy link
Copy Markdown
Contributor Author

@marcotuts yeah, I thought I had done so in my last PR but apparently had not.

@singingwolfboy

Copy link
Copy Markdown
Contributor

Looks good. Technically, the result that you get from the gettext call is translated, so your variable names are inaccurate -- but that makes no difference to the functionality of this PR. 👍

@sarina

sarina commented Oct 3, 2013

Copy link
Copy Markdown
Contributor Author

@singingwolfboy good point. I changed the names of those vars from translated_foo_message to full_foo_message for clarity. Thanks for the review!

sarina added a commit that referenced this pull request Oct 3, 2013
@sarina
sarina merged commit 66b8e1f into master Oct 3, 2013
@sarina
sarina deleted the sarina/more-ins-dash branch October 3, 2013 19:58
Agrendalath pushed a commit to open-craft/openedx-platform that referenced this pull request Oct 19, 2018
…-bump

MCKIN-8582 Version bump for drag and drop
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