[DI] Default undefined env to empty string during compile #28838
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stof
reviewed
Oct 12, 2018
src/Symfony/Component/DependencyInjection/Compiler/ValidateEnvPlaceholdersPass.php
Show resolved
Hide resolved
This was referenced Oct 12, 2018
|
To further clarify, this makes the |
nicolas-grekas
approved these changes
Oct 17, 2018
fabpot
approved these changes
Oct 17, 2018
|
Thank you @ro0NL. |
fabpot
added a commit
that referenced
this pull request
Oct 17, 2018
…(ro0NL) This PR was squashed before being merged into the 4.1 branch (closes #28838). Discussion ---------- [DI] Default undefined env to empty string during compile | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #28827 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Instead of using `null` for undefined envs, use `""` instead. We already default the type to string, so actually providing a string value makes sense. During runtime it will always be string also. Commits ------- 38a8ab9 [DI] Default undefined env to empty string during compile
Merged
nicolas-grekas
added a commit
that referenced
this pull request
Dec 1, 2018
…o0NL) This PR was squashed before being merged into the 4.3-dev branch (closes #28858). Discussion ---------- [DI] Deprecated using env vars with cannotBeEmpty() | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes-ish | New feature? | yes | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | yes | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #28827 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Continuation of #28838 for 4.2 Using environment variables for nodes marked `cannotBeEmpty()` is semantically not possible, we'll never know the value is empty yes/no during compile time. Neither we should assume one or another. Commits ------- 397c19e [DI] Deprecated using env vars with cannotBeEmpty()
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of using
nullfor undefined envs, use""instead. We already default the type to string, so actually providing a string value makes sense. During runtime it will always be string also.