Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Documented the loadForEnv() method#10626

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@javiereguiluz
Copy link
Member

Fixes#10607.

..versionadded::4.2
The ``Dotenv::loadForEnv()`` method was introduced in Symfony 4.2.

You should never store a ``.env`` file in your code repository as it might
Copy link
Member

Choose a reason for hiding this comment

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

This sentence should be remove in favor of the paragraph you introduced, right?

the app on your local machine. The other two optional files (``.env`` +
"environment name" + ``.local`` and ``.env`` + "environment name") work
similarly, but they are specific to some environment.

Copy link
Member

@dunglasdunglasNov 5, 2018
edited
Loading

Choose a reason for hiding this comment

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

May I suggest to add the following:

Suggested change
``DotEnv::loadForEnv()`` implements the exact same behavior than Ruby's `dotenv library`_ and `Create React App's .env support`_. These projects provide extensive documentation about environment variables handling, that is worth reading.

Tiriel and ogizanagi reacted with thumbs up emoji

.. _Packagist:https://packagist.org/packages/symfony/dotenv
.. _twelve-factor applications:http://www.12factor.net/
.. _`app config recommendations`:https://12factor.net/config
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. _`app config recommendations`:https://12factor.net/config
.. _`app config recommendations`:https://12factor.net/config
.. _`dotenv library`:https://github.com/bkeepers/dotenv
.. _`Create React App's .env support`:https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables#what-other-env-files-can-be-used

Tiriel reacted with thumbs up emoji
$dotenv->loadForEnv('dev', __DIR__.'/.env');
// Looks for env vars in these files (and in this order):
// .env.dev.local
// .env.local
Copy link
Member

@dunglasdunglasNov 5, 2018
edited
Loading

Choose a reason for hiding this comment

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

Will be loadedafter.env.dev

Copy link
Contributor

Choose a reason for hiding this comment

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

Will be loaded after .env.dev

Hmm. Are you sure? I didn't check the code, but if it's true it's not respecting the same priorities ashttps://github.com/bkeepers/dotenv/blob/master/README.md#what-other-env-files-can-i-use, i.e:

  • .env.[env].local
  • .env.local
  • .env.[env]
  • .env

Also, what about using the same presentation asbkeepers/dotenv? (The table with complementary information)

Copy link
Contributor

@ogizanagiogizanagiNov 5, 2018
edited
Loading

Choose a reason for hiding this comment

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

Buthttps://facebook.github.io/create-react-app/docs/adding-custom-environment-variables#what-other-env-files-can-be-used does load.env.local after.env.[env]...

IMHO, loading.env.local before.env.[env] makes more sense, asx.local shouldalways allow override. Either you use.env.local to override for all environments, either you use.env.[env].local for a specific env. But.env and.env.[env] will only contains sensible defaults, so.env.[env] should not have precedence over.env.local.

// .env

The ``.env`` file should be committed to the shared repository and contains the
default env var values for the app. The optional ``.env.local`` file shouldn't
Copy link
Member

@dunglasdunglasNov 5, 2018
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
default env var values for the app. The optional ``.env.local`` file shouldn't
default env var values for the app. The``.env`` file should contain development-only settings. It **must not** contain any settings used in production. They are placeholder values helping during the onboarding process. Theoptional ``.env.local`` file shouldn't

ogizanagi reacted with thumbs up emoji
// .env.test.local
// .env.test
// .env

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Similarly to ``DotEnv::load()``, ``DotEnv::loadForEnv()`` never overwrites existing environment variables, nor variables defined by files loaded previously. For instance, if the same variable is defined in both `.env` and `.env.test.local`, the value will be the one defined in `.env.test.local`, because this file is loaded first by ``DotEnv::loadForEnv()``.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add somewhere something similar tohttps://github.com/bkeepers/dotenv/blob/master/README.md#can-i-use-dotenv-in-production last paragraph:

please note that env vars that are general to all environments should be stored in .env. Then, environment specific env vars should be stored in .env.<that environment's name>.

i.e no need to repeat all env vars. Only the ones likely to be specific for the target env.

Copy link
Contributor

@ogizanagiogizanagi left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this! ❤️

$dotenv->loadForEnv('dev', __DIR__.'/.env');
// Looks for env vars in these files (and in this order):
// .env.dev.local
// .env.local
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be loaded after .env.dev

Hmm. Are you sure? I didn't check the code, but if it's true it's not respecting the same priorities ashttps://github.com/bkeepers/dotenv/blob/master/README.md#what-other-env-files-can-i-use, i.e:

  • .env.[env].local
  • .env.local
  • .env.[env]
  • .env

Also, what about using the same presentation asbkeepers/dotenv? (The table with complementary information)

// .env.test.local
// .env.test
// .env

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add somewhere something similar tohttps://github.com/bkeepers/dotenv/blob/master/README.md#can-i-use-dotenv-in-production last paragraph:

please note that env vars that are general to all environments should be stored in .env. Then, environment specific env vars should be stored in .env.<that environment's name>.

i.e no need to repeat all env vars. Only the ones likely to be specific for the target env.

@weaverryan
Copy link
Member

Seesymfony/recipes#481

But, those changes I believe also apply to 3.4

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedNov 10, 2018
edited
Loading

Should be updated to account for #29129
Technically, the method is new to 4.2
But practically, the described behavior (loading .env < .env.local < .env.$env < .env.$env.local) works for new projects starting with 3.4 yes

@weaverryan
Copy link
Member

And I think there are / will be a few more recipe changes still? Once those finish, we can finish this up as quickly as possible

@weaverryan
Copy link
Member

See#10680 - it does not replace this PR (at least not entirely) as in#10680 I don't describe the actual newloadForEnv() method (though I do describe its behavior, which, thanks to the recipe changes, is available to any Symfony project in any version).

@nicolas-grekas
Copy link
Member

Note that the method changed to loadEnv() with different signature and behavior.

@javiereguiluz
Copy link
MemberAuthor

I'm closing this because the handling of .env files is still "a moving target" and too many things are still changing. Let's work on this again after things settle down. Thanks!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

2 more reviewers

@dunglasdunglasdunglas left review comments

@ogizanagiogizanagiogizanagi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

6 participants

@javiereguiluz@weaverryan@nicolas-grekas@dunglas@ogizanagi@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp