Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] framework.php_errors.log now accept a log level#26504
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
[FrameworkBundle] framework.php_errors.log now accept a log level#26504
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedMar 13, 2018
would be great to turn the |
d71b26f to5b6a4feCompareSimperfit commentedMar 14, 2018
@nicolas-grekas I have changed this PR, could you please guide me on how to write test for this ? |
nicolas-grekas 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.
for tests, please check the existing ones, I don't have specific ideas on the topic sorry
| $definition =$container->findDefinition('debug.debug_handlers_listener'); | ||
| if (!$config['log']) { | ||
| if (\is_bool($config['log']) &&!$config['log']) { |
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.
if (!$config['log']) { is enough:0 also disables the logger
| $definition->replaceArgument(1,null); | ||
| } | ||
| if (\is_int($config['log'])) { |
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.
if (\is_int($config['log']) && $config['log']) {
| ->addDefaultsIfNotSet() | ||
| ->children() | ||
| ->booleanNode('log') | ||
| ->scalarNode('log') |
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.
maybe forbid anything else than a bool|int?
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.
done
5b6a4fe tob02c7d7CompareSimperfit commentedMar 26, 2018
Status: Needs Review |
b02c7d7 toc713dcaCompareSimperfit commentedMar 26, 2018
Tests Fixed. |
| ->treatNullLike($this->debug) | ||
| ->validate() | ||
| ->ifTrue(function ($v) {return !(\is_int($v) ||\is_bool($v)); }) | ||
| ->thenInvalid('The "php_errors.log" parameter should be either a int or a boolean.') |
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.
either an integer or a boolean?
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.
@fabpot updated.
c713dca to556c6c1Compare| ->treatNullLike($this->debug) | ||
| ->validate() | ||
| ->ifTrue(function ($v) {return !(\is_int($v) ||\is_bool($v)); }) | ||
| ->thenInvalid('The "php_errors.log" parameter should be either a integer or a boolean.') |
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.
an integer :)
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.
This is good now :p.
556c6c1 to664f821Comparefabpot commentedMar 27, 2018
Thank you@Simperfit. |
…a log level (Simperfit)This PR was merged into the 4.1-dev branch.Discussion----------[FrameworkBundle] framework.php_errors.log now accept a log level| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets |#26267 <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | todo <!-- required for new features -->We are testing that `framework.php_errors.log` is either a bool or an int (set the value of the log level you want).Thisfixes#26267, so it gives a way to not log some level on production.Commits-------664f821 [FrameworkBundle] framework.php_errors.log now accept a log level
…yrixx)This PR was merged into the 4.1-dev branch.Discussion----------[FrameworkBundle] Fixed configuration of php_errors.log| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#26504 and#26740 (wrong implementation)| License | MIT| Doc PR |Commits-------8e0fcf9 [FrameworkBundle] Fixed configuration of php_errors.log
| ->children() | ||
| ->booleanNode('log') | ||
| ->scalarNode('log') | ||
| ->info('Use the app logger instead of the PHP logger for logging PHP errors.') |
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.
is this really descriptive now?
mathieu-pousse commentedMay 23, 2019
Hi, This PR looks great to me, but I am not sure to ensure the aim. In any case, I think this should be explain in the documentation |
SublimeRaiser commentedJan 9, 2020
Here is a link to the docs:https://symfony.com/doc/current/reference/configuration/framework.html#log |
Uh oh!
There was an error while loading.Please reload this page.
We are testing that
framework.php_errors.logis either a bool or an int (set the value of the log level you want).Thisfixes#26267, so it gives a way to not log some level on production.