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.

Sign up for GitHub

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

Jump to bottom

chore: remove bok-choy settings #33350

Merged
merged 44 commits into from
Oct 31, 2023
Merged

chore: remove bok-choy settings #33350

merged 44 commits into from
Oct 31, 2023

Conversation

salman2013
Copy link
Contributor

@salman2013 salman2013 commented Sep 27, 2023 •
edited
Loading

Description

In this PR i have removed the bok-choy settings and all bok-choy references as per the following ticket

https://github.com/orgs/openedx/projects/55/views/1?pane=issue&itemId=37830749

@salman2013 salman2013 marked this pull request as ready for review September 27, 2023 12:02
@salman2013 salman2013 changed the title chore: remove bok choy settings chore: remove bok-choy settings Sep 27, 2023
feanil
feanil reviewed Sep 28, 2023
View reviewed changes
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

Some initial questions and comments.

common/djangoapps/terrain/stubs/lti.py Outdated Show resolved Hide resolved
common/djangoapps/terrain/stubs/lti.py Outdated Show resolved Hide resolved
docs/decisions/0004-managing-django-settings.rst Show resolved Hide resolved
openedx/features/content_type_gating/tests/test_access.py Outdated Show resolved Hide resolved
openedx/testing/coverage_context_listener/pytest_plugin.py Outdated Show resolved Hide resolved
pavelib/paver_tests/test_js_test.py Outdated Show resolved Hide resolved
pavelib/utils/envs.py Outdated Show resolved Hide resolved
pavelib/utils/envs.py Outdated Show resolved Hide resolved
xmodule/modulestore/tests/mongo_connection.py Show resolved Hide resolved
@salman2013 salman2013 requested a review from feanil October 3, 2023 04:48
feanil
feanil reviewed Oct 3, 2023
View reviewed changes
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

Looking really good! Thanks for all the progress so far, I've got some more questions.

openedx/testing/coverage_context_listener/pytest_plugin.py Outdated Show resolved Hide resolved
openedx/testing/coverage_context_listener/pytest_plugin.py Outdated Show resolved Hide resolved
pavelib/utils/envs.py Outdated Show resolved Hide resolved
pavelib/utils/test/suites/js_suite.py Outdated Show resolved Hide resolved
farhan
farhan requested changes Oct 9, 2023
View reviewed changes
common/static/common/js/karma.common.conf.js Outdated Show resolved Hide resolved
pavelib/utils/envs.py Outdated Show resolved Hide resolved
@salman2013 salman2013 requested review from feanil and farhan October 10, 2023 05:55
farhan
farhan requested changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@farhan farhan left a comment

Choose a reason for hiding this comment

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

1 point, rest seems alright

pavelib/utils/envs.py Outdated Show resolved Hide resolved
@salman2013 salman2013 requested a review from timmc-edx October 24, 2023 03:55
feanil
feanil approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

Changes look good to me. I'm gonna merge this Monday so that we don't ruin anyone's weekend if we find that they're using the bokchoy settings in a way we don't expect.

@feanil feanil merged commit 3003abb into openedx:master Oct 31, 2023
62 checks passed
robrap
robrap reviewed Oct 31, 2023
View reviewed changes
pavelib/prereqs.py
@@ -354,13 +354,3 @@ def install_prereqs():
def log_installed_python_prereqs():
""" Logs output of pip freeze for debugging. """
sh("pip freeze > {}".format(Env.GEN_LOG_DIR + "/pip_freeze.log"))


def print_devstack_warning(): # lint-amnesty, pylint: disable=missing-function-docstring
Copy link
Contributor

Choose a reason for hiding this comment

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

@feanil @salman2013: Is there a quick fix to revert this change, or should I revert the whole thing? We could also temporarily remove the call to print_devstack_warning(), but I'd have the question as to whether this warning should really go away? In all cases, what is going to be the quick fix since this is breaking things? Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR was reverted in #33630 until this issue can be fixed. The changes did make it to Prod (temporarily), so other code seems to be ok.

@robrap robrap mentioned this pull request Oct 31, 2023
Merged
@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.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robrap robrap robrap left review comments

@feanil feanil feanil approved these changes

@farhan farhan Awaiting requested review from farhan

@dianakhuang dianakhuang Awaiting requested review from dianakhuang

@timmc-edx timmc-edx Awaiting requested review from timmc-edx

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@salman2013 @edx-pipeline-bot @feanil @dianakhuang @robrap @farhan @timmc-edx