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

Use log package from authd #315

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Use log package from authd #315

merged 2 commits into from
Jan 17, 2025

Conversation

adombeck
Copy link
Contributor

@adombeck adombeck commented Jan 14, 2025

For consistency and because it's more convenient (especially the Debugf, Infof etc. methods are more convenient to use than slog.Debug(fmt.Sprintf(...))).

UDENG-5773
UDENG-4523

TODO

@adombeck
Copy link
Contributor Author

The failure of the code sanity job is tracked in https://warthogs.atlassian.net/browse/UDENG-5774

@adombeck adombeck force-pushed the UDENG-5773-use-authd-log-pkg branch from 565b989 to 55a6388 Compare January 14, 2025 15:51
@adombeck adombeck marked this pull request as ready for review January 14, 2025 15:53
@adombeck adombeck requested a review from a team as a code owner January 14, 2025 15:53
This was referenced Jan 14, 2025
Copy link
Member

@didrocks didrocks left a comment

Choose a reason for hiding this comment

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

I didn’t spot anything wrong. Nice work! I agree this is easier than slog.Foo(fmt.Sprintf())

@adombeck adombeck merged commit 377c8c2 into main Jan 17, 2025
3 of 4 checks passed
@adombeck adombeck deleted the UDENG-5773-use-authd-log-pkg branch January 17, 2025 10:13
@3v1n0
Copy link
Collaborator

3v1n0 commented Jan 17, 2025

Great thing... Indeed it's a more consistent move.

Wondering if even this package could be somewhat shared in a private package... As I don't feel it would be fine in authd.log (unless we want to be committed to it...)

@adombeck
Copy link
Contributor Author

Wondering if even this package could be somewhat shared in a private package...

Yes, @didrocks and I briefly discussed that a few days ago, with the result that we both think it could make sense to move this to a separate repo but we don't want to do that right now.

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.

3 participants