thephpleague / tactician-logger

Adds PSR-3 logging support to the Tactician Command Bus
62 stars 8 forks source link

Implement logger as a listener (as well) #2

Closed sagikazarmark closed 9 years ago

sagikazarmark commented 9 years ago

It is just an idea based on the structure of the logger, the middleware architecture is probably still better.

It seems to be very similar to the event middleware. It has the three events, exception handling.

You could implement the whole middleware as one listener.

The benefits:

Again, it is just an idea, but the logger as a listener seems to be much simpler. Probably we could create both.

rosstuck commented 9 years ago

I toyed with the idea once this form took shape but it couples this package to the events package for a fairly small amount of code. As we discussed way back when at the start of the event package, I'm a fan of having it readily available but I want to explore alternate extensibility techniques in PHP. Also, the Events package relies on League\Event as well and people get all fussy about using things other than their preferred event dispatchers.

I'm not purely satisfied with the LogLevel setup either but it's reasonably simple for end users. I suspect those with really specific logging needs will simply implement their own middleware anyways.

So, I like the idea but as mentioned elsewhere, I'm ok with the small scale duplication (especially across packages).

sagikazarmark commented 9 years ago

I am thinking about implementing this logic as a listener in the command events package. What do you think?

My objections: I would have to set psr/log as a dependency which I don't want to. Maybe a command-listeners package where we can collect these type of listeners?

Also, I was thinking about creating an event package based on symfony event dispatcher. I don't like it, but many people uses. On the contrary, I hope @frankdejonge will come up with an Event PSR quickly. Then it wouldn't be needed.

rosstuck commented 9 years ago

Reimplementing the logging logic in the event package? That seems a bit redundant and confusing for users. :frowning: Command Listeners sounds pretty vague, I'd rather have use-cased based plugins. I know this package isn't how you'd implement it but I'd prefer to have a single canonical logger in Tactician core.

I for one would welcome our new Event PSR Overlords for command-events but that's a discussion for that package. :grin:

sagikazarmark commented 9 years ago

Hey, just brain dump...

rosstuck commented 9 years ago

No offense meant, sorry it came across as a bit short. :) On Mar 17, 2015 9:41 PM, "Márk Sági-Kazár" notifications@github.com wrote:

Hey, just brain dump...

— Reply to this email directly or view it on GitHub https://github.com/thephpleague/tactician-logger/issues/2#issuecomment-82588957 .