Skip to content

Revert "saleem-latif/WL-328: Multi-Site Comprehensive Theming" - #12116

Merged
mattdrayer merged 1 commit into
rc/2016-04-12from
mattdrayer/rc2016-04-12/revert-theming
Apr 12, 2016
Merged

Revert "saleem-latif/WL-328: Multi-Site Comprehensive Theming"#12116
mattdrayer merged 1 commit into
rc/2016-04-12from
mattdrayer/rc2016-04-12/revert-theming

Conversation

@mattdrayer

Copy link
Copy Markdown
Contributor

@rlucioni @andy-armstrong @saleem-latif -- FYI, here is the revert PR for the new multi-site comprehensive theming feature. I have replicated the problem on a sandbox (mattdrayer.sandbox.edx.org) and will continue to troubleshoot. In the meantime let's see what Jenkins has to say about this change set.

This reverts commit a796b56.

Conflicts:
lms/envs/common.py
openedx/core/djangoapps/theming/models.py
pavelib/assets.py
pavelib/paver_tests/test_assets.py

@rlucioni

Copy link
Copy Markdown
Contributor

@mattdrayer are there other commits on the RC that rely on this commit?

@mattdrayer

Copy link
Copy Markdown
Contributor Author

I'm not aware of any -- I think I've been able to resolve the conflicts with some of the changes that have been made to the asset watchers (post-merge)

@rlucioni

Copy link
Copy Markdown
Contributor

@mattdrayer got it, thanks. Looks like you left a conflict marker in the Python: https://github.com/edx/edx-platform/pull/12116/files#diff-e753ceed3e8176f6e597fc95dac4993bR60. You'll have to re-run the tests after removing it.

@rlucioni

Copy link
Copy Markdown
Contributor

@mattdrayer for the record, I'm in favor of reverting this to give your team a chance to debug this feature in a more relaxed setting. Running a patch release once this issue is resolved is an option; it would make getting this out the door easier to manage.

@mattdrayer
mattdrayer force-pushed the mattdrayer/rc2016-04-12/revert-theming branch 2 times, most recently from 3607195 to bb65082 Compare April 11, 2016 18:36
@mattdrayer
mattdrayer force-pushed the mattdrayer/rc2016-04-12/revert-theming branch 3 times, most recently from fd0e761 to 02fd7f8 Compare April 12, 2016 03:01
@saleem-latif

Copy link
Copy Markdown
Contributor

@mattdrayer , @rlucioni , @andy-armstrong I was able to recreate the issue on sandbox (theme-test.sandbox.edx.org). I did so by applying "edx.org" theme without removing COMPREHENSIVE_THEME_DIR from lms.env.json and cms.env.json and the
same error message appeared. So, my guess is that, on stage, COMPREHENSIVE_THEME_DIR is set to /edx/app/edxapp/edx-platform/themes/edx.org instead of /edx/app/edxapp/edx-platform/themes/. So most probably COMPREHENSIVE_THEME_DIR was not removed from /edx/app/edxapp/lms.env.json and  /edx/app/edxapp/cms.env.json

This reverts commit a796b56.

Conflicts:
	lms/envs/common.py
	openedx/core/djangoapps/theming/models.py
	pavelib/assets.py
	pavelib/paver_tests/test_assets.py
@mattdrayer
mattdrayer force-pushed the mattdrayer/rc2016-04-12/revert-theming branch from 02fd7f8 to 392ee7d Compare April 12, 2016 12:22
@mattdrayer
mattdrayer merged commit 9a362d9 into rc/2016-04-12 Apr 12, 2016
@mattdrayer
mattdrayer deleted the mattdrayer/rc2016-04-12/revert-theming branch June 14, 2016 17:03
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