Skip to content

Conversation

@bpolaszek
Copy link
Contributor

@bpolaszek bpolaszek commented Dec 2, 2021

The current implementation does not permit storing the default address to listen to, as it is hardcoded.

By using $_SERVER instead of getenv() to check for the X_LISTEN env var, one could use a dotenv library such as symfony/dotenv to read .env files and populate X_LISTEN.

This could help devs override the default port to listen to (in case something else already use 127.0.0.1:8080 in their projects), and override the default port in their .env.local.

Example here: https://github.com/bpolaszek/freddie/blob/main/.env.test#L11 (this actually doesn't work and I had to specify the "real" env var in the CI)

@clue clue added the new feature New feature or request label Dec 10, 2021
Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@bpolaszek Thank you for looking into this, changes LGTM! :shipit:

We've discussed this in our team with the pros and cons of the various ways to access environment variables in PHP and I agree that the $_SERVER variable should be supported out of the box!

I'll use this as a starting point for a future follow-up PR to see how we can also take changes to $_ENV and getenv() into account.

Keep it up! 👍

@clue clue merged commit 5350ea2 into clue:main Dec 10, 2021
@bpolaszek bpolaszek deleted the refactor/server-arguments branch December 10, 2021 13:11
@SimonFrings SimonFrings added this to the v0.6.0 milestone Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants