-
Notifications
You must be signed in to change notification settings - Fork 45
Log errors by default #197
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
Conversation
@vadorovsky looks like eff4440 accidentally lost logging if an explicit level was not passed to the CLI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, happy to see the custom error
function to be gone and replaced with anyhow! So far it looks good, I'm just going to try it a bit before approving.
Go ahead and merge when you're satisfied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm but let's let @vadorovsky try what he wants to try.
As per comment I wouldn't add a new dep tho.
src/bin/bpf-linker.rs
Outdated
error(&e.to_string(), clap::error::ErrorKind::Format); | ||
} | ||
} | ||
either::for_both!(subscriber, s => tracing::subscriber::set_global_default(s))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless it's already in the dep tree, it doesn't seem worth adding a new dep just for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at Cargo.lock; it's already in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a dev dependency right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, removed.
Remove imperative `clap::Error::exit`.
Remove imperative
clap::Error::exit
.