[Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() #35308
Conversation
e44c3a7 to
19cd11c
Compare
19cd11c to
d66bcd9
Compare
cd1b59d to
3c3f05a
Compare
| private $usePutenv; | ||
| private $envKey; | ||
| private $debugKey; | ||
| private $prodEnvs = ['prod']; |
There was a problem hiding this comment.
perhaps for consistency, define $testEnvs = ['test'] + $defaultEnv = 'dev'; as well? Then cleanup the arg list in loadEnv()
There was a problem hiding this comment.
I considered that, but I think it's not worth the upgrade trouble.
There was a problem hiding this comment.
it would avoid a half stateful / stateless API, and perhaps is worth it from the component's design POV.
There was a problem hiding this comment.
do we prefer mutability? or what about new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']) :/
There was a problem hiding this comment.
We have no issues with mutability when its purpose is to configure the instance.
Using many constructor args is a pain when you only want to change the last one.
Setters are better here IMHO.
There was a problem hiding this comment.
i undertand the technical aspect, im curious if a user should change the context between loading envs (vs using a dedicated instance per context). In practice the context is a fixed concern.
There was a problem hiding this comment.
In practice, people will figure out what they need :) I considered alternatives before deciding for this design: it looks the best compromise to me.
There was a problem hiding this comment.
IMHO it's component design first :) i strongly feel the right design is
$dotenv = new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']);
$dotenv->load('.env', $putenv = true);
$dotenv->loadAll(['.env'], $putenv = true);but im fine either way, my only use case is a generated one :)
There was a problem hiding this comment.
I understand, that's what I meant by "compromise". We're not designing from the ground up, and in practice this is the most seamless.
3c3f05a to
d66ecbe
Compare
|
ping @symfony/mergers |
35c22bc to
e91d110
Compare
|
Rebase needed |
…ling Dotenv::loadEnv()
e91d110 to
98c7d30
Compare
|
Thank you @nicolas-grekas. |
….php before calling Dotenv::loadEnv() (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | - | License | MIT | Doc PR | - The goal of this PR is to eventually get rid of the `config/bootstrap.php` file in Symfony 5.1 apps. I think we've done enough iterations on that piece of bootstrapping logic to put it inside the `Dotenv` component. This fully replaces https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/4.2/config/bootstrap.php It doesn't conflict with current apps so they'll be fine keeping the `config/bootstrap.php` file until they're upgraded. The new bootstrapping logic will require adding this line in `bin/console` and `public/index.php`: ```php (new Dotenv())->bootEnv(dirname(__DIR__).'/.env'); ``` Recipes updated at symfony/recipes#724 Commits ------- 98c7d30 [Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv()
This PR was merged into the 1.5-dev branch. Discussion ---------- Add forward compat with Dotenv v5.1 Sidekick of symfony/symfony#35308 Commits ------- 1bf83d4 Add forward compat with Dotenv v5.1
The goal of this PR is to eventually get rid of the
config/bootstrap.phpfile in Symfony 5.1 apps.I think we've done enough iterations on that piece of bootstrapping logic to put it inside the
Dotenvcomponent.This fully replaces https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/4.2/config/bootstrap.php
It doesn't conflict with current apps so they'll be fine keeping the
config/bootstrap.phpfile until they're upgraded.The new bootstrapping logic will require adding this line in
bin/consoleandpublic/index.php:Recipes updated at symfony/recipes#724