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

Update logger to facilitate the use of global logger #150

Merged
merged 8 commits into from
Oct 31, 2024
Merged

Conversation

rawmind0
Copy link
Contributor

@rawmind0 rawmind0 commented Oct 29, 2024

This PR add the chance to use global zap logger within the packages. Also some code cleanup has been done, like remove the logger mutex due to zap package already take care of them

🔗 zboto Link

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2024

CLA assistant check
All committers have signed the CLA.

@rawmind0 rawmind0 requested review from jleni and lucaslopezf October 29, 2024 14:49
Copy link
Member

@emmanuelm41 emmanuelm41 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the improvement. I would improve a bit the readme though. Maybe using chat gpt, providing code as input?

pkg/runner/runner.go Outdated Show resolved Hide resolved
pkg/cli/root.go Show resolved Hide resolved
@rawmind0
Copy link
Contributor Author

I really like the improvement. I would improve a bit the readme though. Maybe using chat gpt, providing code as input?

Sure, we may try this with cursor?? would be pretty nice. I've just added a pretty basic README

@emmanuelm41
Copy link
Member

@rawmind0 yes, using cursor is what I meant!!

@rawmind0
Copy link
Contributor Author

Ok, let me check if i can do it.

@rawmind0
Copy link
Contributor Author

Logger README has been updated using ia

## Installation

```go
go get -u go.uber.org/zap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go get zap? I don't think this is correct!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Fixed

@emmanuelm41 emmanuelm41 merged commit f526f8c into main Oct 31, 2024
4 checks passed
@emmanuelm41 emmanuelm41 deleted the logger branch October 31, 2024 16:53
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.

4 participants