-
Notifications
You must be signed in to change notification settings - Fork 4.2k
refactor: Clean up CMS production settings cruft #36306
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
refactor: Clean up CMS production settings cruft #36306
Conversation
e0e2246 to
accbbec
Compare
|
@feanil This is ready for review. I've tested using dump_settings, and I'll smoke test with the PR sandbox on Monday. |
|
Sandbox deployment successful 🚀 |
We separate out the handful of settings which have useful comments. The rest of the settings' comments were not helpful--they were either just stating the obvious, or they were duplicative of what's documented in common.py.
699251b to
16d4177
Compare
|
Sandbox deployment successful 🚀 |
|
Smoke test looks good. @feanil ready for review |
feanil
left a comment
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.
PR looks good, checked for typos and logical errors and didn't spot anything new.
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
This is a pure refactoring of cms/envs/production.py, removing several redundant statements that have accrued over the years as the platform moved from python-only, to python+json, to python+json+yaml, to today's python+yaml setup. This is the CMS version of: * a81493c * (originally 1593923) Also included: * Add some more explicit structure to the both LMS's and CMS's production.py using big comments. * In both LMS and CMS settings, alphabetize the production overrides, and remove the extraneous comments. Separate out the handful of settings which have useful comments. The rest of the settings' comments were not helpful--they were either just stating the obvious, or they were duplicative of what's documented in common.py. Co-Authored-By: Feanil Patel <feanil@axim.org> Part of: openedx#36215
This is a pure refactoring of cms/envs/production.py, removing several redundant statements that have accrued over the years as the platform moved from python-only, to python+json, to python+json+yaml, to today's python+yaml setup. This is the CMS version of: * a81493c * (originally 1593923) Also included: * Add some more explicit structure to the both LMS's and CMS's production.py using big comments. * In both LMS and CMS settings, alphabetize the production overrides, and remove the extraneous comments. Separate out the handful of settings which have useful comments. The rest of the settings' comments were not helpful--they were either just stating the obvious, or they were duplicative of what's documented in common.py. Co-Authored-By: Feanil Patel <feanil@axim.org> Part of: #36215
Description
What it says on the tin. See individual commits for more details
Supporting Information
Part of:
This is essentially the CMS version of:
Testing instructions
With an up-to-date version of both master and this PR's branch, run
./diff_settings.sh master kdmccormick/settings-cms-envs-production, wherediff_settings.shis:Should yield: