[FrameworkBundle] framework.php_errors.log now accept a log level #26504
Conversation
|
would be great to turn the |
Simperfit
force-pushed
the
feature/add-a-parameter-for-log-level
branch
from
d71b26f to
5b6a4fe
Compare
|
@nicolas-grekas I have changed this PR, could you please guide me on how to write test for this ? |
Simperfit
changed the title
nicolas-grekas
left a comment
nicolas-grekas
left a comment
There was a problem hiding this comment.
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.
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.
if (\is_int($config['log']) && $config['log']) {
| ->addDefaultsIfNotSet() | ||
| ->children() | ||
| ->booleanNode('log') | ||
| ->scalarNode('log') |
There was a problem hiding this comment.
maybe forbid anything else than a bool|int?
Simperfit
force-pushed
the
feature/add-a-parameter-for-log-level
branch
from
5b6a4fe to
b02c7d7
Compare
|
Status: Needs Review |
Simperfit
force-pushed
the
feature/add-a-parameter-for-log-level
branch
from
b02c7d7 to
c713dca
Compare
|
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.') |
Simperfit
force-pushed
the
feature/add-a-parameter-for-log-level
branch
from
c713dca to
556c6c1
Compare
| ->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.
This is good now :p.
Simperfit
force-pushed
the
feature/add-a-parameter-for-log-level
branch
from
556c6c1 to
664f821
Compare
|
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 <!-- see https://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). This fixes #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.
is this really descriptive now?
|
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 |
Here is a link to the docs: https://symfony.com/doc/current/reference/configuration/framework.html#log |
7 participants
We are testing that
framework.php_errors.logis either a bool or an int (set the value of the log level you want).This fixes #26267, so it gives a way to not log some level on production.