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.

Sign up for GitHub

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

Jump to bottom

logging: switch to using log/slog #138

Merged
merged 1 commit into from
Aug 17, 2023
Merged

logging: switch to using log/slog #138

merged 1 commit into from
Aug 17, 2023

Conversation

sethvargo
Copy link
Contributor

No description provided.

@sethvargo sethvargo requested a review from a team as a code owner June 13, 2023 19:12
@sethvargo sethvargo force-pushed the sethvargo/slog branch 3 times, most recently from e422f72 to 4f42ab7 Compare June 30, 2023 20:37
@sethvargo sethvargo force-pushed the sethvargo/slog branch 3 times, most recently from 5f42083 to 88ce9b2 Compare July 23, 2023 14:20
@sethvargo sethvargo force-pushed the sethvargo/slog branch 3 times, most recently from eead4c6 to 1d400c9 Compare August 9, 2023 03:11
@sethvargo sethvargo changed the title exp: add experimental slog packages logging: switch to using log/slog Aug 9, 2023
@sethvargo sethvargo force-pushed the sethvargo/slog branch 2 times, most recently from 8ab312b to 58785b0 Compare August 9, 2023 03:21
@sethvargo sethvargo mentioned this pull request Aug 9, 2023
Merged
@sethvargo
Copy link
Contributor Author

Looks like golangci-lint still isn't quite happy...

@sethvargo sethvargo removed the request for review from spencergoldberg August 9, 2023 03:44
yolocs
yolocs reviewed Aug 9, 2023
View reviewed changes
logging/logger.go Outdated Show resolved Hide resolved
logging/logger.go Outdated Show resolved Hide resolved
@sethvargo sethvargo force-pushed the sethvargo/slog branch 5 times, most recently from 0683374 to 828b7cb Compare August 11, 2023 18:58
@sethvargo
logging: switch to using log/slog
a6b4a9c 
This changes a few interfaces of the logging library, but maintains as much compatability as possible in switching to the standard library log/slog. The new APIs are designed to allow for a more programatic configuration of the logger, instead of always relying on the environment.
@sethvargo
Copy link
Contributor Author

@yolocs okay so for rollout plan here, since this contains breaking changes, I think we should:

  1. Cut a new release of pkg
  2. Merge this
  3. Cut a new release of pkg and note the breaking changes
  4. Start opening PRs to update deps and change the syntax where needed

Thoughts?

@yolocs
Copy link
Contributor

yolocs commented Aug 17, 2023

@yolocs okay so for rollout plan here, since this contains breaking changes, I think we should:

  1. Cut a new release of pkg
  2. Merge this
  3. Cut a new release of pkg and note the breaking changes
  4. Start opening PRs to update deps and change the syntax where needed

Thoughts?

sgtm, along (3) give team a headsup

yolocs
yolocs approved these changes Aug 17, 2023
View reviewed changes
Copy link
Contributor

@yolocs yolocs left a comment

Choose a reason for hiding this comment

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

LGTMing to move forward

@sethvargo sethvargo merged commit def8ffd into main Aug 17, 2023
@sethvargo sethvargo deleted the sethvargo/slog branch August 17, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yolocs yolocs yolocs approved these changes

@bradegler bradegler Awaiting requested review from bradegler bradegler is a code owner automatically assigned from abcxyz/infrastructure-team

@sailorlqh sailorlqh Awaiting requested review from sailorlqh sailorlqh is a code owner automatically assigned from abcxyz/infrastructure-team

@raserva raserva Awaiting requested review from raserva

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sethvargo @yolocs