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

Allow full range of log levels #119

Merged
merged 10 commits into from
Dec 12, 2022
Merged

Allow full range of log levels #119

merged 10 commits into from
Dec 12, 2022

Conversation

teabot
Copy link
Contributor

@teabot teabot commented Dec 12, 2022

Motivation

Sometimes info and debug is not enough.

Changes

  • Removed -d flag from CLI
  • Added -l flag to permit usage of any one of the rs/zerolog levels
  • Defaults to info as before
  • Added test cases

@teabot
Copy link
Contributor Author

teabot commented Dec 12, 2022

Thanks for the review @andrasbeni — applied your suggestions.

Use: "oxia",
Short: "Short description",
Long: `Long description`,
PreRunE: configureLogLevel,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also call the common.ConfigureLogger() from this hook

// LogDebug Used for flags
LogDebug bool
// LogLevel Used for flags
LogLevel zerolog.Level
Copy link
Collaborator

Choose a reason for hiding this comment

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

(for a different PR) Should we also improve the LogJson argument to make it a kind of --log-format json|text|.... ?

Co-authored-by: Dave Maughan <[email protected]>
@merlimat merlimat merged commit 86895b9 into streamnative:main Dec 12, 2022
@merlimat merlimat deleted the all-the-log-levels branch December 12, 2022 22:46
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