search

bitExpert / prophiler-psr7-middleware

[DEPRECATED] Prophiler PSR7 Middleware
Apache License 2.0
7 stars 8 forks source link

Pass a psr-3 logger , probably a null logger #10

Closed harikt closed 8 years ago

harikt commented 8 years ago

Hi,

Do you think it is ok, if we can pass a psr-logger in constructor than using LoggerFactory::getLogger(__CLASS__);

https://github.com/bitExpert/prophiler-psr7-middleware/blob/master/src/bitExpert/Http/Middleware/Psr7/Prophiler/ProphilerMiddleware.php#L39

In that case we can remove the require of bitexpert/slf4psrlog from composer.json and add psr/log instead.

Thoughts ?

shochdoerfer commented 8 years ago

Actually I did this on purpose :) I do not like the idea of passing logger instances to classes since I see a logger not a dependency like a service or a repository that needs to be injected. A logger is part of the infrastructure and should be treated differently. That was the reason for building bitexpert/slf4psrlog. Some more insights can be found in the corresponding blog post.

Do you have issues with the current behaviour? What do you want to achieve?

harikt commented 8 years ago

Hey @shochdoerfer ,

Thank you for your quick comments. I was basically trying to have a skelton for expressive ( usage for me only for the current time ) which only needs some portion of this class.

I don't want many loggers installed. So do you think I can copy some portions of this class and give credits to this repo?

Thank you

msphn commented 8 years ago

Both projects slf4psrlog and this middleware are released under the apache license. https://github.com/bitExpert/slf4psrlog/blob/master/LICENSE

harikt commented 8 years ago

@cookiekiller sometimes I really know all these license and what it permits / compatibility with other license. Eg : if I release as MIT.

shochdoerfer commented 8 years ago

@harikt ok got it. Technically the bitexpert/slf4psrlog package is not logger so this should not be a problem at all, it is just one more dependency ;) - All you would need is to add 3 - 5 lines of config code to your bootstrapping file to let the package use your existing logger.

In a perfect world I would love to see you using the package as-it-is but if you have serious concerns you can for sure copy the code and refer to his repo.

harikt commented 8 years ago

In a perfect world I would love to see you using the package as-it-is but if you have serious concerns you can for sure copy the code and refer to his repo.

Yes. I do agree and wish the same. That is why I was asking you whether you are willing to drop it in favour of null logger. But sometimes we really don't need more dependency for just a class.

I will see what I can do.

Thank you.

shochdoerfer commented 8 years ago

@harikt technically the call of LoggerFactory::getLogger() returns a NullLogger so you are good to go, it just comes with a dependency you do not need ;)

harikt commented 8 years ago

:)