-
Notifications
You must be signed in to change notification settings - Fork 60
Hotfix: Four minor issues to address before release #9
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
Conversation
|
@bradenmacdonald verified on sandbox 👍; however, description for fix 4 (default value of "max_attempts" ) is confusing - I thought it would automatically change max_attempts in studio editor, while it just does not update max_attempts at all anymore. |
|
@sarina These are our last changes to problem builder as required by Harvard. We would like to get them onto edx.org as soon as possible, though it looks like we've missed this week's release unfortunately. Can you please take a quick look at this and if it looks good, I'll merge it and then update the hash in https://github.com/edx/edx-platform/pull/7638 and get your approval to merge that as well. Let me know if you want an OSPR. |
|
Braden - I'm still getting caught up from Pycon. Can I take a look On Mon, Apr 13, 2015 at 12:21 PM, Braden MacDonald <notifications@github.com
|
|
Done: https://openedx.atlassian.net/browse/OSPR-530 Thanks @sarina |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're very allergic to !important - is there a way you can write without this? !important overrides everything on the page and is strongly discouraged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can rewrite this to use a longer (more specific) selector, so that it can still override the existing style in the LMS that's causing the issue here.
|
@bradenmacdonald thanks for the description and laying out the fixes. I share @e-kolpakov 's feedback about how the fourth fix works, but I don't think it's terrible, just maybe a bit unexpected from your description. |
|
@sarina OK, I have removed the use of |
|
@bradenmacdonald LGTM. 👍 |
|
Thanks @sarina ! |
Hotfix: Four minor issues to address before release


Description: The
edx-releasebranch contains only the code that has been reviewed by both OpenCraft and edX for release on edX.org. In testing that release branch, we found a few minor issues which this PR addresses.Fixes:
Fixed with 45072bb
Fixed with 3d4bf29
Fixed with 8bf362d
Fixed with 0d85a3f
Once these fixes are reviewed and approved here, I am also planning to cherry-pick them onto our master (development) branch.
Sandbox: http://sandbox2.opencraft.com:18010/