Fix typos in code #4422
Fix typos in code #4422
Conversation
|
What do you think about removing the parameters entirely and putting the class names directly into the service definitions? There is no benefit in using the parameters. |
|
Following the Symfony best practices it would make sense to remove the parameters. |
|
👍 looks good |
| namespace Acme\DemoBundle\EventListener; | ||
|
|
||
| use Symfony\Component\Console\Event\ConsoleTerminateEvent; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| class ConsoleExceptionListener | ||
| class ConsoleTerminateListener |
There was a problem hiding this comment.
This renaming is wrong IMO. ConsoleExceptionListener describes better the goal of the listener than ConsoleTerminateListener. The responsibility of the class is to log exceptions
There was a problem hiding this comment.
A much better name could be ConsoleErrorLoggerListener (or ErrorLoggerListener to be shorter), given that it does not really log exceptions, but failure exit codes (logging exceptions would be on using the console.exception event)
There was a problem hiding this comment.
ping @ifdattic It would be great if you could fix this, then we can merge these changes in!
|
I agree. @wouterj or @weaverryan can merge this once they have some time. Thank you for your great job as always @ifdattic! |
|
Glad to help and happy being a part of a wonderful community |
|
Yes, excellent change! Well, I don't see any difference with the class re-name, but I like the new name anyways :). But the removing of the constants is of course perfect. Thanks Andrew! |
This PR was merged into the 2.3 branch. Discussion ---------- Fix typos in code | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3 | Fixed tickets | PS 1: As so far I used only yaml files for services not sure if the changes to XML and PHP code blocks are correct PS 2: Maybe I configured something wrong or something like that (or you could point me in the right direction for more information), but I don't think the terminate listener will work. In my opinion when command is exited with exception the exit code should be more than zero, but on `ConsoleTerminateEvent` and even on `ConsoleExceptionEvent` the exit code is `0` Commits ------- 2135b82 Change: ConsoleTerminateListener => ErrorLoggerListener fc403b1 Removed parameters from service d4ba3c0 Fix typos in code
PS 1: As so far I used only yaml files for services not sure if the changes to XML and PHP code blocks are correct
PS 2: Maybe I configured something wrong or something like that (or you could point me in the right direction for more information), but I don't think the terminate listener will work. In my opinion when command is exited with exception the exit code should be more than zero, but on
ConsoleTerminateEventand even onConsoleExceptionEventthe exit code is0