Skip to content

[Config] [Configuration] Add $_ENV and $_SERVER example with .env #17735

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

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Jan 13, 2023

Fixes #17693

@carsonbot carsonbot added this to the 5.4 milestone Jan 13, 2023
@OskarStark OskarStark changed the title [Configuration] Add $_ENV and $_SERVER example with .env [Configuration] Add $_ENV and $_SERVER example with .env Jan 15, 2023
@OskarStark OskarStark requested a review from xabbuh January 16, 2023 07:13
@xabbuh
Copy link
Member

xabbuh commented Jan 16, 2023

Is it really a common use case to access environment variables through the superglobal arrays? I usually reference them inside my service configuration with %env(...)% instead (values are injected into services this way instead of having code depend on global state).

@alexandre-daubois
Copy link
Member Author

I wouldn't say it is "common" but this is something that can happen, especially when using the Dotenv component as a standalone. But I may add a note to inform that best practices is to inject env vars through %env()% maybe?

@xabbuh
Copy link
Member

xabbuh commented Feb 3, 2023

I think I would put it the other way around: Show examples how to make use of the env processor in the DI config and add a short sentence explaining that you could also access the value through the superglobal arrays.

@alexandre-daubois
Copy link
Member Author

I moved the paragraph below the examples with %env()% and reworded a bit to indicates that env vars may be accessed in the code. Is it better? 🙂

@carsonbot carsonbot changed the title [Configuration] Add $_ENV and $_SERVER example with .env [Config] [Configuration] Add $_ENV and $_SERVER example with .env Feb 10, 2023
@javiereguiluz javiereguiluz merged commit cd192a5 into symfony:5.4 Feb 11, 2023
@javiereguiluz
Copy link
Member

Thanks Alex!

Similar to Christian, I was a bit concerned about this. So, while merging I "demoted" this explanation to a note and added a message to warn readers that they should really not use this in Symfony apps. See 3a4b017

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

Successfully merging this pull request may close these issues.

4 participants