Improve Log

[zolotarev-om]

• added ability to initialize monolog-update instance
• added ability to specify external monolog handler for methods of initializing the logs
• initialize* methods now initialize only monolog instance, init*Log methods now initialize each specific log
• simplify FQN / fix PHPDOC

[noplanman]

Hi @zolotarev-om

Oh noes, just saw you closed this!
Sorry for my late response, but I was literally looking at the PR and thinking a lot about it right now.

Thanks so much for your contribution, I like the idea of your PR, to bring more order and flexibility to the logger 👍

I understand your thinking when you added a parameter to the init* methods, the $path parameter becomes useless though.

A more logical (and more testable) approach in my view, would be to add separate methods entirely for the external initialisers. (e.g. initExternalErrorLog, initExternalDebugLog, initExternalUpdateLog), what do you think?

As parameters, a user woud have to pass at least a Handler and optionally a Logger. That way, we could also make the initialize methods private, as they don't do anything on their own anyway.

@MBoretto @jacklul
I kind of forgot why we are using 2 Logger instances in this class (1 for debug and error, 1 for update).
Mind refreshing my memory please? Or can we merge them into one?

[zolotarev-om]

The closing PR fortunately by accident. Moved fork into the organization and accidentally deleted branch.

I like the idea of initExternal*Log methods. Happy to use it in my PR. But I think it is not correct to use in each of initExternal*Log Logger instanse(even optional), because it will override and override private property that client-side it is not obvious.

Unfortunately, at the moment I don't understand the reason to use two instances of the Logger. You may want to use a single instance? Then we can use 1 public initilize method with optional external Logger instance.

[noplanman]

You may want to use a single instance?

I remember there was a good reason to use 2 separate ones, but I honestly don't remember, as they're using different log levels anyway 😕

The way it's set up now, the external Logger is only set once and can't be overridden, so the private variable will stay the same. Doesn't mean this is good though!

Thinking of this, that was the idea of the initialize method in the first place, to just pass a Logger instance that already has the handlers set, making it unnecessary to call the init* methods.
So I guess it's about finding a middle line, making it as logical as possible.

Also, maybe it would make more sense to call the methods set*, and also allow re-setting/overwriting a handler?

[noplanman]

Right, if I recall correctly, we decided to have the two, to make sure that the update logs are separately handled.

But I'm just questioning if this still makes sense...

[noplanman]

@zolotarev-om Have you thought about this any further?
@MBoretto Do you remember why it was decided to use 2 different logging instances?

[akalongman]

In the package should be a single logger instance. Log level can be defined in each log call

[noplanman]

Using PSR-3 (#964) in favour of this.

Thanks @zolotarev-om anyway!