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

Added Yii::getEnv #16162

Closed
wants to merge 1 commit into from
Closed

Added Yii::getEnv #16162

wants to merge 1 commit into from

Conversation

leandrogehlen
Copy link
Contributor

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes

Helper to retrieve values from enviroment variables

@samdark
Copy link
Member

samdark commented Apr 25, 2018

Is it better than any existing dotenv libraries?

@leandrogehlen
Copy link
Contributor Author

The problem is get the value from environment variables.
These libraries just load the environmnet variables from file

@samdark
Copy link
Member

samdark commented Apr 25, 2018

Nope. These usually do both i.e. environment variables + files with files priority.

@leandrogehlen
Copy link
Contributor Author

I guess that https://github.com/vlucas/phpdotenv is the most popular and used in laravel.
And i didn't find this feature.

Laravel has implemented the same feature
https://github.com/laravel/framework/blob/5.6/src/Illuminate/Support/helpers.php#L604

@samdark
Copy link
Member

samdark commented Apr 25, 2018

yiisoft/yii-base-web#1

@samdark
Copy link
Member

samdark commented Apr 25, 2018

I don't see pros of having it then.

  1. Default value is easy to achieve: getenv('TEST') ?: 'default'.
  2. Stripping quotes and converting string values is weird. For example, you may want to pass string value empty and won't be able to.

@samdark samdark added this to the 2.1.0 milestone Apr 25, 2018
@leandrogehlen
Copy link
Contributor Author

You are right.

@samdark samdark closed this Apr 25, 2018
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.

2 participants