Skip to content
This repository was archived by the owner on Jan 7, 2020. It is now read-only.

replace logger with logrus for logging #623

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

suhussai
Copy link
Contributor

@suhussai suhussai commented Jan 1, 2017

Description

Removes the logger package and related tests.

Related Issue

#615

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Example logs with logrus:
logrus log output

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-io
Copy link

codecov-io commented Jan 1, 2017

Current coverage is 22.79% (diff: 10.76%)

Merging #623 into master will decrease coverage by 0.22%

@@             master       #623   diff @@
==========================================
  Files            26         25     -1   
  Lines          1972       2009    +37   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            454        458     +4   
- Misses         1463       1496    +33   
  Partials         55         55          

Powered by Codecov. Last update d8d8863...6b697d2

@suhussai suhussai force-pushed the logrus_logging_refactoring branch from b51d438 to 6b697d2 Compare January 4, 2017 04:40
@palourde
Copy link
Contributor

palourde commented Jan 6, 2017

Thank you @suhussai for this PR, I really appreciate it!

The only problem I see so far with the logrus package is the fact that there's no way, out of the box, to identify the Remote Caller (see sirupsen/logrus#63) but it's an issue with the package itself, not your implementation.

Removing that capability would make debugging much more harder, so we'll have to think of a workaround before merging this change. I'll look around, let me know if you find something!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants