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

[FrameworkBundle][Secrets] Hook configured local dotenv file #34922

Merged

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Dec 10, 2019

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #34905
License MIT
Doc PR -

Configured local_dotenv_file does not currently substitute the secrets.vault service definition first argument value, rendering this configuration option useless + we don't need to set defaults in secrets.xml since everything is overriden in FrameworkExtension with the same default values (from the configuration).

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I duplicated them for clarity, but maybe this is better this way. Good catch for the fix.

@nicolas-grekas
Copy link
Member

A test case needs love.

@fancyweb fancyweb force-pushed the fwb-hook-secrets-local-dotenv-file branch from 829f3d5 to 4016fea Compare December 11, 2019 12:22
@fancyweb
Copy link
Contributor Author

Done.

@nicolas-grekas nicolas-grekas force-pushed the fwb-hook-secrets-local-dotenv-file branch from 4016fea to 56f542c Compare December 13, 2019 12:07
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

nicolas-grekas added a commit that referenced this pull request Dec 13, 2019
…le (fancyweb)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[FrameworkBundle][Secrets] Hook configured local dotenv file

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #34905
| License       | MIT
| Doc PR        | -

Configured local_dotenv_file does not currently substitute the secrets.vault service definition first argument value, rendering this configuration option useless + we don't need to set defaults in secrets.xml since everything is overriden in FrameworkExtension with the same default values (from the configuration).

Commits
-------

56f542c [FrameworkBundle][Secrets] Hook configured local dotenv file
@nicolas-grekas nicolas-grekas merged commit 56f542c into symfony:4.4 Dec 13, 2019
@fancyweb fancyweb deleted the fwb-hook-secrets-local-dotenv-file branch December 13, 2019 12:17
This was referenced Dec 19, 2019
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.

None yet

3 participants