I noticed that you fixed the fatal crash that occurs when using a different Logger in #2954559: Use logger interface to add support for other loggers, however this fix was inadvertently reverted in #2941795: Uninstalling module wipes out unrelated display configs..

CommentFileSizeAuthor
#2 svg_image-issue-reverted-code-fix-loggerinterface-3008518-2-8.x.patch1.58 KBsvenryen

Comments

svenryen created an issue. See original summary.

svenryen’s picture

svenryen commented
StatusFileSize
new svg_image-issue-reverted-code-fix-loggerinterface-3008518-2-8.x.patch1.58 KB

Here's the patch :)

imyaro’s picture

imyaro commented
Assigned: Unassigned » imyaro

  • zvse committed 8fec29f on 8.x-1.x 
    Issue #3008518 by svenryen, zvse: Issue #2941795 reverted the code fix...
imyaro’s picture

imyaro commented
Status: Active » Fixed

Hello,
Nice catch!
But it will be better to use there LoggerChannelInterface instead of the LoggerInterface.
Committed fix

pdenooijer’s picture

pdenooijer commented

Sorry, but changing to the LoggerChannelInterface still will break external loggers that only use the PSR LoggerInterface. So why change it to the LoggerChannelInterface and remove this option? Further more I don't see the pro's of including the LoggerChannelInterace, all the newly added methods are just for internal use. The LoggerInterface has all the methods you need anyway, seeing your only use the `error` message anyway.
https://en.wikipedia.org/wiki/Interface_segregation_principle

Please change it to the requested LoggerInterface.

pdenooijer’s picture

pdenooijer commented
Status: Fixed » Needs work
imyaro’s picture

imyaro commented
Status: Needs work » Fixed

@pdenooijer you are right.

Changed to the LoggerInterface. Please check the dev branch.

pdenooijer’s picture

pdenooijer commented

Thanks, then I can remove the patch!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.