search

vishen / go-chromecast

cli for Google Chromecast, Home devices and Cast Groups
Apache License 2.0
824 stars 79 forks source link

Logrus- configure with timestamp, update fmt.Println* calls, possibly remove logrus #54

Open titpetric opened 4 years ago

titpetric commented 4 years ago

Hello,

particularly I have the issue with the following line in application.go:

fmt.Println("state", status.PlayerState, "status.Idle
Reason", status.IdleReason)

The output itself doesn't even go through logrus, which is what happens if you'd replace fmt.Println with log.Println. Replacing it however gives the following output:

INFO[0000] state IDLE status.IdleReason INTERRUPTED     
INFO[0001] state BUFFERING status.IdleReason            
INFO[0002] state PLAYING status.IdleReason              
INFO[0003] state PLAYING status.IdleReason              
INFO[0012] state PLAYING status.IdleReason

Without timestamp information (as is the default with stdlib log package).

Importing stdlib "log" and issuing stdlog.Println as a work around also doesn't work (no output, buffered, or thrown away somehow, didn't investigate further).

I'd suggest throwing away logrus honestly, I don't see a strong need for anything beyond stdlib logging. Do you need structured logging particularly (I don't exactly see go-chromecast piping structured logs into ELK or something...).

vishen commented 4 years ago

I am not sure I follow regarding :

fmt.Println("state", status.PlayerState, "status.IdleReason", status.IdleReason)

I can't see this line in application.go?

I am not opposed to removing logrus from most of the project, I don't think it adds anything and I would rather have a timestamp rather than the log level. I think logrus is used more in ui/ but I suspect that should also be replaceable.

titpetric commented 4 years ago

Sorry, I may have added the fmt line myself 😬 definitely missing the timestamps however.

On Fri, 3 Jan 2020 at 23:10, Jonathan Pentecost notifications@github.com wrote:

I am not sure I follow regarding :

fmt.Println("state", status.PlayerState, "status.IdleReason", status.IdleReason)

I can't see this line in application.go?

I am not opposed to removing logrus from most of the project, I don't think it adds anything and I would rather have a timestamp rather than the log level. I think logrus is used more in ui/ but I suspect that should also be replaceable.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vishen/go-chromecast/issues/54?email_source=notifications&email_token=AABY7EHYW6EPBLKCMWEQFALQ36ZWFA5CNFSM4KCRGDX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICGL6Y#issuecomment-570713595, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABY7EDPFXWC6EAQKALQ5P3Q36ZWFANCNFSM4KCRGDXQ .