Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

chore: update bok-choy hostname #1217

Merged

Conversation

salman2013
Copy link
Contributor

Description:

In this PR i update the BOK_CHOY_HOSTNAME to SERVER_HOSTNAME in the scope of removing bok-choy from edx-platform (openedx/public-engineering#13) ticket:https://github.com/orgs/openedx/projects/55/views/1?pane=issue&itemId=37830749, the reason to update the variable name is to intact the logic to choose browser for karma and remove all bok-choy setting either by name or functionality.

@salman2013 salman2013 requested a review from feanil October 27, 2023 03:59
@@ -5,7 +5,7 @@ services:
command: bash -c 'cd /edx/app/edxapp/edx-platform && source ../edxapp_env && while true; do paver watch_assets --w=$$ASSET_WATCHER_TIMEOUT; sleep 2; done'
container_name: "edx.${COMPOSE_PROJECT_NAME:-devstack}.lms_watcher"
environment:
BOK_CHOY_HOSTNAME: edx.devstack.lms_watcher
SERVER_HOSTNAME: edx.devstack.lms_watcher
Copy link
Contributor

Choose a reason for hiding this comment

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

SERVER_HOSTNAME is very vague and generic, is there a better name we could use to describe what server we're referring to? Is this the LMS_SERVER? the FRONTEND_TEST_SERVER? KARMA_HOSTNAME? something else? Can you come up with a more descriptive name for this variable?

@salman2013 salman2013 requested a review from feanil October 30, 2023 04:47
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.

The port names should have the same prefix if they're related.

docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@salman2013 salman2013 requested a review from feanil October 31, 2023 10:03
@feanil feanil merged commit b528524 into openedx-unsupported:master Oct 31, 2023
11 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants