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 logrus integration #107

Closed
wants to merge 4 commits into from
Closed

Conversation

rbUUbr
Copy link
Contributor

@rbUUbr rbUUbr commented Jan 26, 2020

  1. Added logrus for this project
  2. I've added flag for rootCmd with log-level, but seems it does not work :( could you please check it? Because I found out that other flags for rootCmd also doesn't work

closes #78

@rbUUbr rbUUbr requested a review from Arkweid Jan 26, 2020
@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Jan 27, 2020

Awesome work. I will check it.

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Feb 2, 2020

Great idea with loggerClient, but I don't sure now we can use Logrus as a library. I went through docs and looks like Logrus can be configured only by levels, but we need flexible logs groups which may be combined in any positions. Am I mistaken?
#78 (comment)

@rbUUbr
Copy link
Contributor Author

@rbUUbr rbUUbr commented Feb 2, 2020

hm, interesting, I thought, that we can different log levels as log groups. for example, the summary will have an info log level, failures will have an error log level. Maybe you're right, but as for me (as for active user of Lefthook), it is not too interesting to output just failures, but interesting to get some general information with failures. Can we do this? or how do you see this feature at all?

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Feb 3, 2020

I see it like this:

// somewhere in the code:
loggerClient.Meta(au.Cyan("SYNCING"), au.Bold("lefthook.yml"))

// in logger client we checking config key `output`
if outputMetaEnabled {
        log.Println(args...)
}

For failure/success keys we store output from processes in pseudo terminal and write in after command ended with some exit code

@rbUUbr
Copy link
Contributor Author

@rbUUbr rbUUbr commented Feb 3, 2020

yep, that's it! for info log level you will receive errors also, but if you will set log level to error or to warn, you will not get meta information output, that's the way how we can combine outputs. It is imo, we can do it through yml config like here #78 (comment)

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Feb 3, 2020

If you sure we can made it with logrus ok. Could you make example in this PR?

@rbUUbr
Copy link
Contributor Author

@rbUUbr rbUUbr commented Feb 3, 2020

hm, at first, I think, it will be very good to solve issues that I've provided in general comment, because they're some kind of blockers for this :D

I've added flag for rootCmd with log-level, but seems it does not work :( could you please check it? Because I found out that other flags for rootCmd also doesn't work

could you please check it?

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Feb 4, 2020

could you please check it?

Ok! I will check it. Just a little busy during the workdays 😅

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented May 14, 2020

Sorry for so long @rbUUbr !
I have cheked it and logrus not fit well our desires. For this task we don't need log-level.
We only just need to mark every output with their purpose in bloks. Like it described here:
#78 (comment)
For example:

loggerClient.Meta(au.Cyan("Lefthook v" + version))

loggerClient.Summary(au.Cyan("\nSUMMARY:"), au.Brown("(SKIP EMPTY)"))

@Arkweid
Copy link
Collaborator

@Arkweid Arkweid commented Aug 3, 2020

Solved.

@Arkweid Arkweid closed this Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants