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

log: Clean log package & move config to log package #3677

Merged
merged 3 commits into from
Feb 24, 2020

Conversation

lukedirtwalker
Copy link
Collaborator

@lukedirtwalker lukedirtwalker commented Feb 24, 2020

Cleans up the public API of the log package and
moves the log config from env to the log package.

Breaking: this commit changes the config format to snake_case.
Furthermore the APIs change:

  • The toml key for logging config is changed from logging to log
  • env.SetupLogging is replaced by log.Setup
  • env.Logging is replaced by log.Config
  • util.GetDebugID() is now log.NewDebugID()
  • log.RandId() is removed and log.NewDebugID() should be used instead
  • The log packages no longer provides flag setup, whoever wants to use flags should define them in main.

Fixes #3594


This change is Reviewable

Cleans up the public API of the log package and
moves the log config from env to the log package.

Breaking: this commit changes the config format to snake_case.
Furthermore the APIs change:
- The toml key for logging config is changed from `logging` to `log`
- `env.SetupLogging` is replaced by `log.Setup`
- `env.Logging` is replaced by `log.Config`
- `util.GetDebugID()` is now `log.NewDebugID()`
- `log.RandId()` is removed and `log.NewDebugID()` should be used instead
- The log packages no longer provides flag setup, whoever wants to use flags should define them in main.

Fixes scionproto#3594
Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

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

Reviewed 100 of 100 files at r1, 1 of 5 files at r2.
Reviewable status: 97 of 101 files reviewed, 1 unresolved discussion (waiting on @karampok and @lukedirtwalker)


go/lib/log/logtest/config.go, line 1 at r1 (raw file):

// Copyright 2019 Anapaya Systems

2020

Copy link
Collaborator Author

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: 97 of 101 files reviewed, 1 unresolved discussion (waiting on @karampok)


go/lib/log/logtest/config.go, line 1 at r1 (raw file):

Previously, karampok (Konstantinos) wrote…

2020

Done.

Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 5 files at r2, 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@lukedirtwalker lukedirtwalker added the i/breaking change PR that breaks forwards or backwards compatibility label Feb 24, 2020
@lukedirtwalker lukedirtwalker merged commit 82be069 into scionproto:master Feb 24, 2020
@lukedirtwalker lukedirtwalker deleted the pubLogRefactor branch February 24, 2020 19:17
stygerma pushed a commit to stygerma/scion that referenced this pull request Mar 26, 2020
Cleans up the public API of the log package and
moves the log config from env to the log package.

Breaking: this commit changes the config format to snake_case.
Furthermore the APIs change:
- The toml key for logging config is changed from `logging` to `log`
- `env.SetupLogging` is replaced by `log.Setup`
- `env.Logging` is replaced by `log.Config`
- `util.GetDebugID()` is now `log.NewDebugID()`
- `log.RandId()` is removed and `log.NewDebugID()` should be used instead
- The log packages no longer provides flag setup, whoever wants to use flags should define them in main.

Fixes scionproto#3594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i/breaking change PR that breaks forwards or backwards compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor go/lib/log into more idiomatic Go
2 participants