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

revert: revert: build: finish replacing paver assets #34701

Merged

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented May 6, 2024

Re-merges:

plus a fix, in its own commit:


fix: fall back to settings.COMPREHENSIVE_THEME_DIRS correctly

There was a logical error in the compile_sass management command:
instead of falling back to settings.COMPREHENSIVE_THEME_DIRS when
--theme-dirs was None or missing, we only fell back to it when
--theme-dirs was missing.

This caused theme compilation to be skipped when COMREHENSIVE_THEME_DIRS
is not set in the environment, even though
settings.COMPREHENSIVE_THEME_DIRS is set in Django settings, which
is currently the case for edx.org.

There was a logical error in the compile_sass management command:
instead of falling back to settings.COMPREHENSIVE_THEME_DIRS when
--theme-dirs was *None or missing*, we only fell back to it when
--theme-dirs was *missing*.

This caused theme compilation to be skipped when COMREHENSIVE_THEME_DIRS
*is not set* in the environment, even though
settings.COMPREHENSIVE_THEME_DIRS *is set* in Django settings, which
is currently the case for edx.org.
@kdmccormick kdmccormick changed the title Kdmccormick/assets final remerge revert: revert: build: finish replacing paver assets May 6, 2024
@kdmccormick kdmccormick merged commit b1393ac into openedx:master May 7, 2024
79 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/assets-final-remerge branch May 7, 2024 12:40
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

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