Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
[ConfigBuilder] Replace all framework config#15269
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ea01165 to4d644d1CompareNyholm commentedApr 21, 2021
Only the config reference and messeger left =) |
a8c5161 toa5f4b3bCompare49f1d9b toacdf609CompareNyholm commentedApr 21, 2021
I cannot figure out why not all errors show in the diff.. I only see the first 5.. |
Uh oh!
There was an error while loading.Please reload this page.
configuration/env_var_processors.rst Outdated
| return static function (ContainerBuilder $container, FrameworkConfig $framework) { | ||
| $container->setParameter('env(TRUSTED_HOSTS)', '10.0.0.1,10.0.0.2'); | ||
| $framework->trustedHosts('%env(csv:TRUSTED_HOSTS)%'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same here.
Uh oh!
There was an error while loading.Please reload this page.
Nyholm commentedApr 22, 2021
Hm... About the CI not showing all errors. It will not showthis error and anything after it. Earlier, it did not showthis error and anything after it. It make sense if there were a special character that was being printed, but I dont see any. But all (Btw Iurlencode the line breaks to have a multi line comment.) |
545a66a to3a31028Comparec270e59 to71c6a6bCompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Nyholm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Assumingsymfony/symfony#40903 will be merged. This PR is ready for a final review.
| ->defaultSerializer('messenger.transport.symfony_serializer') | ||
| ->symfonySerializer() | ||
| ->format('json') | ||
| ->context('foo', 'bar'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do we have a better example than this?
…urator (Nyholm)This PR was merged into the 5.3-dev branch.Discussion----------[Config] Builder: Remove typehints and allow for EnvConfigurator| Q | A| ------------- | ---| Branch? | 5.x| Bug fix? | yes -- maybe| New feature? | no -- maybe =)| Deprecations? | no| Tickets || License | MIT| Doc PR |When [writing documentation](symfony/symfony-docs#15269 (comment)) we found that we don't really support environment variables in the leaves. Ie, we expect a boolean but you provide `"%env(ENABLE_FOO)%"`This PR will also introduce `ParamConfigurator` to allow parameters to be passed as config.The changes to the generated code:```diff /**+ * `@param` bool|ParamConfigurator $value * `@default` false * `@return` $this */- public function enabled(bool $value): self+ public function enabled($value): self { $this->enabled = $value; return $this; }```Commits-------59b79d3 [Config] Builder: Remove typehints and allow for EnvConfigurator
Nyholm commentedApr 26, 2021
Wohoo. The code PR is merged. |
Nyholm commentedApr 29, 2021
Is there something I can do to help this PR to get merged? |
javiereguiluz commentedApr 30, 2021
Sorry it took us some time to merge this. Thanks Tobias! |
aa04e88 toe57adb9CompareNyholm commentedApr 30, 2021
Thank you for merging. I'll fix the rest now |
Uh oh!
There was an error while loading.Please reload this page.
This PR will replace all framework configuration with the new ConfigBuilders.