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

feat: add logger package and tests #3108

Merged
merged 5 commits into from
Oct 17, 2024
Merged

feat: add logger package and tests #3108

merged 5 commits into from
Oct 17, 2024

Conversation

mkcp
Copy link
Contributor

@mkcp mkcp commented Oct 16, 2024

Description

This PR adds a logger package using log/slog and tests. It's extracted from the proof of concept in #3101.

Related Issue

Relates to #2576

Checklist before merging

@mkcp mkcp self-assigned this Oct 16, 2024
@mkcp mkcp requested review from a team as code owners October 16, 2024 17:45
Copy link

netlify bot commented Oct 16, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit a16d2bd
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6711606c7bfef70008ad86c9

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pkg/logger/logger.go 95.65% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/pkg/logger/logger.go 95.65% <95.65%> (ø)

Signed-off-by: Kit Patella <kit@defenseunicorns.com>
Signed-off-by: Kit Patella <kit@defenseunicorns.com>
Signed-off-by: Kit Patella <kit@defenseunicorns.com>
src/pkg/logger/logger.go Show resolved Hide resolved
src/pkg/logger/logger.go Show resolved Hide resolved
schristoff
schristoff previously approved these changes Oct 17, 2024
Signed-off-by: Kit Patella <kit@defenseunicorns.com>
@mkcp
Copy link
Contributor Author

mkcp commented Oct 17, 2024

I reintroduced the package level default logger here rather than passing into slog's default, and added more clarity on how it's intended to be used as a development aid.

When adding a Default().Info() call to init(),
Screenshot 2024-10-17 at 12 08 29

we get a usable default logger:
Screenshot 2024-10-17 at 12 09 47

Copy link
Contributor

@schristoff schristoff left a comment

Choose a reason for hiding this comment

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

beautiful

@mkcp mkcp added this pull request to the merge queue Oct 17, 2024
Merged via the queue into main with commit 39d0812 Oct 17, 2024
26 checks passed
@mkcp mkcp deleted the mkcp/add-logger branch October 17, 2024 19:55
mjnagel pushed a commit to mjnagel/zarf that referenced this pull request Oct 21, 2024
Signed-off-by: Kit Patella <kit@defenseunicorns.com>
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
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