8000 Change the EnvConfigurator to only handle the .env file by dunglas · Pull Request #423 · symfony/flex · GitHub
[go: up one dir, main page]
Skip to content

Change the EnvConfigurator to only handle the .env file #423

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Change the EnvConfigurator to only handle the .env file #423

wants to merge 1 commit into from

Conversation

dunglas
Copy link
Member
@dunglas dunglas commented Sep 21, 2018

Companion PR for symfony/recipes#466 and symfony/symfony#28533.

As explained in symfony/recipes#466 (comment), it will require to tag a v2 to prevent problems with previous installation.

In a next PR, we may want to allow recipes to add values in specific environment env files (such as .env.test. It will require to tweak a bit the Flex server. I'm not sure that it's really needed.

@dunglas dunglas mentioned this pull request Sep 21, 2018
Closed
@maxhelias
Copy link
Contributor

In this case, would not it be more prudent to merge it on a branch dev to prepare a new version ?
Because for this PR, this implies a change of Git workflow for the devs because the advantage of the .env.dist file is that it is versioned.

Or I suggest defining a system like symfony/symfony#28533 to edit all environments and have a recipe structure like this for add values in specific environment :

"env" : {
   "dev": {
      // ...
   },
   "test": {
     // ...
   }
}

We would just have to make conditions on the keys in the env configurator without creating a break change.

WDYT?

@dunglas
Copy link
Member Author
dunglas commented Sep 21, 2018 •
edited
Loading

Because for this PR, this implies a change of Git workflow for the devs because the advantage of the .env.dist file is that it is versioned.

Now it's the .env file that is versioned. The developper can override it by using a .env.local file that isn't versioned. See https://github.com/symfony/recipes/pull/466/files#diff-48a706859e5c9d4e903b5065baa57726R21

Or I suggest defining a system like symfony/symfony#28533 to edit all environments and have a recipe structure like this for add values in specific environment :

This PR already requires symfony/symfony#28533 to be merged! The config you propose would be fine, but in another PR (probably after 4.2, as it requires to modify the Flex server, on the hand the current PR works with the current server).

without creating a break change.

Actually, we need the breaking because of this: symfony/recipes#466 (comment)

@maxhelias maxhelias mentioned this pull request Sep 22, 2018
Closed
@ogizanagi
Copy link
ogizanagi commente 8000 d Oct 28, 2018

Now that symfony/symfony#28533 is merged, we can move forward here and in symfony/recipes#466 :)

@nicolas-grekas
Copy link
Member

I'm not sure we need this PR anymore.
Instead, I'd suggest doing just one thing: when a .env.test file is found, skip updating phpunit.xml files.
And keep all the other behaviors identical.

@nicolas-grekas nicolas-grekas mentioned this pull request Nov 9, 2018
Merged
@dunglas
Copy link
Member Author
dunglas commented Nov 9, 2018

Closing in favor of #434

@dunglas dunglas closed this Nov 9, 2018
tgalopin pushed a commit to tgalopin/flex that referenced this pull request Dec 3, 2020
Merge pull request symfony#423
1fc11f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogizanagi ogizanagi ogizanagi approved these changes

@maxhelias maxhelias maxhelias approved these changes

@fabpot fabpot Awaiting requested review from fabpot

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

@sroze sroze Awaiting requested review from sroze

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dunglas @maxhelias @ogizanagi @nicolas-grekas
0