Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional formatter to writer hook #1184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sivachandran
Copy link

The changes make hooks.Writer to accept optional Formatter which would be used to format the log entries before writing.

@dgsb
Copy link
Collaborator

dgsb commented Sep 30, 2020

Thanks for your contribution. It is interesting indeed. Can you improve the documentation a bit on the hook.Writer class on how to use the new field and what is the expected behaviour when it is not defined ?

@sivachandran
Copy link
Author

@dgsb Added a line about Formatter usage. Hope it helps.

@utdrmac
Copy link

utdrmac commented Oct 29, 2020

When can we expect this to be merged?

@utdrmac
Copy link

utdrmac commented Oct 29, 2020

Here's the difference between 'DisableColors' false/true. Why is there such a total change in the formatting?

ESC[33mWARNESC[0m[2020-10-29T15:25:06Z] Shutting things down...
ESC[36mINFOESC[0m[2020-10-29T15:25:06Z] Database closed
time="2020-10-29T15:25:10Z" level=info msg="Connected to RPC server" Host="http://127.0.0.1:18732"
time="2020-10-29T15:25:10Z" level=debug msg="Loaded Network Constants" BlocksPerCycle=2048 BlocksPerRollSnapshot=256 PreservedCycles=3

Expected when DisableColors: false

WARN[2020-10-29T15:25:06Z] Shutting things down...
INFO[2020-10-29T15:25:06Z] Database closed

@utdrmac
Copy link

utdrmac commented Oct 29, 2020

Seems this is an issue with the formatter itself and not this PR.

} else {

I'll open a different issue

@dgsb
Copy link
Collaborator

dgsb commented Nov 21, 2020

Thanks for your contribution @sivachandran

@someburner
Copy link

Anything holding this up? Works great for me.

@utdrmac
Copy link

utdrmac commented Jan 23, 2021

@someburner What's holding this up is that logrus is, unfortunately, a dead project. It says in the README that new features won't be added. You'll have to do like me and run your own fork.
Yes, also in the README it says they are not dead, but if you're not adding new features, nor accepting community contributions, you're certainly not an "active" project.

someburner added a commit to someburner/logrus that referenced this pull request Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants