Skip to content
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

make asides render in studio #6296

Merged
merged 2 commits into from
Dec 18, 2014
Merged

make asides render in studio #6296

merged 2 commits into from
Dec 18, 2014

Conversation

dmitchell
Copy link
Contributor

Replaces https://github.com/edx/edx-platform/pull/6277 based on master rather than the other branch.

@doctoryes i don't see a thumb from you on either PR

@dmitchell dmitchell force-pushed the dhm/studio_asides branch 2 times, most recently from 6ef6968 to 2318136 Compare December 17, 2014 22:17
from config_models.models import ConfigurationModel


class StudioConfig(ConfigurationModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

So - is this a Studio-wide config? Or asides-only? The docstring above implies asides-only - but this class name implies Studio-wide. The model should be called what it actually is - to avoid confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would have liked to see this name changed.

@dmitchell
Copy link
Contributor Author

@doctoryes addr'd all comments except extraction of common code in tests. can I get a thumb?

@doctoryes
Copy link
Contributor

👍

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.

3 participants