-
Notifications
You must be signed in to change notification settings - Fork 24
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
Is error the right default? #10
Comments
Sounds reasonable.
Personally I'd expect anything up to log level info to show up by default. I feel that's how |
I think that there is no right default log level for all applications (but do think that "error" is only rarely a good default), and it might make sense to have that configurable by the application (eg. using a type parameter). For |
I would suggest closing this as resolved. The |
Yes, since this was opened
I am curious why you say "error is okay as default level." Granted, where I'm coming from for this is that I have historically used logging as my way of sending messages to my users in CLIs which is why "info" was important for my use case. I've started to shift away from that but I could still see warnings being shown by default |
I think UNIX philosophy would say It depends on the tool, its intended users, … |
I was hoping to be able to have this default to Off :/ I took a look at the code to see why there wasn't an option already, and took a stab at adding the option myself. And there is no Is there any way to achieve this type of functionality? My failed attempt: #[derive(Debug)]
pub struct OffLevel;
impl LogLevel for OffLevel {
fn default() -> Option<tracing_log::log::Level> {
Some(tracing_log::log::Level::)
}
} |
Right after I posted this I looked deeper at the code and figured it out! If you are welcome to contributions I could add this to the library to allow defaulting to off? #[derive(Args, Debug)]
pub struct SharedArgs {
#[clap(flatten)]
pub verbosity: clap_verbosity_flag::Verbosity<OffLevel>,
}
#[derive(Debug)]
pub struct OffLevel;
impl LogLevel for OffLevel {
fn default() -> Option<tracing_log::log::Level> {
None
}
} |
In addition to this, I would also like to add to the docs on how to change the default level, it wasn't immediately clear to me how to do this, it took some digging |
While we would welcome the contribution, we need to discuss whether having a
There's this sentence directly on the main page of docs and in README. Not sure, what else you mean by this. But you can make a PR and I can review it. |
To help in supporting an
|
|
I opened a pr #95 for adding the
@pksunkara I saw this sentence, I'm not entirely sure what I was expecting the way to do this to be, but seeing this example just didn't click. I added an example to make it extremely obvious comparing to the default behavior 🙏🏼. I saw there were some suggestions around the Maybe I could just make my example more tailored to adding your own level by implementing the |
If I specify Edit: just realized that |
Right now, only errors are shown by default.
I would assume at least warnings would show up. Probably also info(rmational) messages.
Of course, this requires adding a way to make things quieter (which we might need already, see #9). We'd probably need a
--quiet
/-q
flag to go the other way.The text was updated successfully, but these errors were encountered: