search

pivot-libre / tideman

Implementation of the Tideman ranked pairs algorithm
Apache License 2.0
9 stars 3 forks source link

Added some logging infrastructure #47

Closed carlschroedl closed 7 years ago

carlschroedl commented 7 years ago

I added logging to an existing class, added logging setup to the tests' bootstrap, and added log printing as a build target.

I reviewed the options for injecting logging on Andrew's handy blog post. https://www.futureproofphp.com/2017/03/06/best-way-inject-logger/ I enjoyed the comments section too.

I tried the different approaches. I liked how the SLF4PSR approach:

  1. decouples the code from particular PSR-compliant logger implementations
  2. permits someone to use the tideman library without a dependency injection framework, and without the busy work of constructing and passing in loggers to many instances.
  3. permits someone uninterested in logging to quickly specify the PSR NullLogger in one place.

The caveats are that this doesn't seem to be the approach that is widely used in the PHP community, and it requires an additional dependency on the standard logging facade package, which, while small, and not in imminent need of any features, doesn't have a lot of activity on GitHub.

I'm wondering if we could clear up at least two points of ignorance on my part.

Do we know at this point whether it will be easy to integrate this into the Laravel app? I'm guessing the answer is "no, let's try it!", but we can certainly take another approach )

Is there something that I'm not grasping that makes the injection-based logging more attractive? I haven't used a dependency injection framework in PHP for a long time -> Symfony, 5 years ago.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 64.667% when pulling be9c73de4b82cfb8c3c6abfe5b350e5912bf1b66 on carlschroedl:addLogging#45 into d2a92285ab6fa70f6d2c70f93f31f9a3187078f9 on pivot-libre:0.x.

carlschroedl commented 7 years ago

Thanks! I kinda lost track of this.