Skip to content

Consistently use display_name_with_default in Studio - #7496

Merged
sarina merged 1 commit into
openedx:masterfrom
open-craft:studio-display-name-consistency
Mar 27, 2015
Merged

Consistently use display_name_with_default in Studio#7496
sarina merged 1 commit into
openedx:masterfrom
open-craft:studio-display-name-consistency

Conversation

@bradenmacdonald

Copy link
Copy Markdown
Contributor

A one-line fix for Studio + acceptance test.

Description: If an XBlock has no display_name set but instead implements display_name_with_default, Studio sometimes uses the latter and sometimes just uses the block type instead. This is a one-line fix to address that inconsistency.

Discussions: Discussed with Cale and Ned at open-craft/xblock-mentoring#4 (comment)

Sandbox: Active at http://sandbox.opencraft.com:18010/ though if it's working it should be indistinguishable from normal.

Partner information: hosted on edx.org (Harvard)

Merge timeline: TBD... but hoping this can get in quickly as it is a fix used by the mentoring v2 block that is almost finished its review.

Notes: Not sure if we want a full bok choy test for this, but I wrote one and am including it for now. The test does fail without this fix.

@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @bradenmacdonald! I've created OSPR-472 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.

@sarina

sarina commented Mar 27, 2015

Copy link
Copy Markdown
Contributor

👍 from me.

@cpennington or @nedbat - would one of you be able to quickly review this?

@cpennington

Copy link
Copy Markdown
Contributor

👍

@sarina

sarina commented Mar 27, 2015

Copy link
Copy Markdown
Contributor

One question before I Merge - if a block implements display_name but NOT display_name_with_default, what will happen? I just want to make sure this change isn't going to inadvertently break anything.

@bradenmacdonald

Copy link
Copy Markdown
Contributor Author

@sarina There is a default implementation of display_name_with_default mixed into all blocks, and it will use display_name if available.

@sarina

sarina commented Mar 27, 2015

Copy link
Copy Markdown
Contributor

@bradenmacdonald ah awesome - thanks for the clarification.

sarina added a commit that referenced this pull request Mar 27, 2015
Consistently use display_name_with_default in Studio
@sarina
sarina merged commit 68ba178 into openedx:master Mar 27, 2015
@bradenmacdonald
bradenmacdonald deleted the studio-display-name-consistency branch March 27, 2015 18:58
@sarina sarina added the open-source-contribution PR author is not from Axim or 2U label Apr 7, 2015
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.

4 participants