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

feat: PHP SPX #820

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: PHP SPX #820

wants to merge 3 commits into from

Conversation

SamJUK
Copy link
Contributor

@SamJUK SamJUK commented Nov 17, 2024

Supporting work to make use of the new PHP SPX images introduced in wardenenv/images#17

PR for Supporting documentation: wardenenv/docs#25

Key Points:

  • New boolean feature flag WARDEN_PHP_SPX to toggle feature state
  • Feature flag added to each .env init template, with a default of disabled
  • Update warden shell to automatically pick the correct container depending on the feature flag

@@ -7,7 +7,9 @@ loadEnvConfig "${WARDEN_ENV_PATH}" || exit $?
## set defaults for this command which can be overridden either using exports in the user
## profile or setting them in the .env configuration on a per-project basis
WARDEN_ENV_SHELL_COMMAND=${WARDEN_ENV_SHELL_COMMAND:-bash}
WARDEN_ENV_SHELL_CONTAINER=${WARDEN_ENV_SHELL_CONTAINER:-php-fpm}

WARDEN_ENV_DEFAULT_SHELL_CONTAINER=$([ $WARDEN_PHP_SPX == 1 ] && echo "php-spx" || echo "php-fpm")
Copy link
Member

Choose a reason for hiding this comment

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

@SamJUK I don't fully agree with this. The warden shell goes to the standard php-fpm container and warden debug goes to php-debug container. To me this would seem like we could either add a flag --spx or create a new warden spx command to drop to a shell in it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also prefer having warden spx command, similar to warden debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants