idno / known

A social publishing platform.
https://withknown.com/opensource
Other
1.05k stars 196 forks source link

Consider using monolog (or equivalent) for Logging #1133

Closed kylewm closed 7 years ago

kylewm commented 8 years ago

Maybe it's my newness to the php ecosystem, but logging via error_log feels limited. \Idno\Core\Logging helps by adding some context. I wonder if we should consider replacing it with the PSR-3 compatible Monolog.

I think this would be nice for a few reasons:

benwerd commented 8 years ago

I think this is a great idea, tbh. Being able to leverage other projects like this rather than reinventing wheels sounds like the way to go. Will take a deeper look.

benwerd commented 8 years ago

A good middle ground could be to establish \Idno\Core\Logging as an effective wrapper for Monolog, should we want to change it out in the future.

kylewm commented 8 years ago

I'm having second thoughts about this.

I'm not convinced that deferring to a logging framework is a good idea for every user ... it could lead to a situation where you have to check the Known log and the error log to track down issues, and it's already hard enough to explain to people where the error log might be.

For now I think it would be worthwhile to make \Idno\Core\Logging implement \Psr\Log\LoggerInterface, and to make Monolog an optional plugin (maybe the plugin's init would replace \Idno\Core\Logging with its own \Monolog\Logger). Would also give us some time to evaluate whether it's actually a good thing for core.

ipranjal commented 8 years ago

+1 having abstract classes to be extending function is the way i support third party integration in my software mixed up with Dependency injection

ipranjal commented 8 years ago

Dependency Injection is not a cup of tea for known but there should be abstract classes to extend some sort of core functionality like logging via a plugin . calling abstract classes instead of main class increases scope of changes without our rewriting the whole core . For example lets say in some far future known want to render classes via a futuristic tempting engine and not Bonita . This would be a lot of rewrite either in core of known or the tempting engine , but having abstract classes as a middleware solve this kind of thrones in development and extension. Let the main class implement the abstract class

mapkyca commented 7 years ago

Closing... I think the abstraction which has already been done is sufficient to enable a new logging backend if there's a specific need...