-
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
default to log level info #11
Conversation
/// warnings, `-vvv` enables info logging, `-vvvv` debug, and `-vvvvv` | ||
/// trace. | ||
#[structopt(long = "quiet", short = "q")] | ||
quiet: bool, |
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.
Another option is to allow repeated --quiet. It might not be obvious that to be completely quiet you need -qq
.
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.
Oh yeah, that's a cool suggestion! If that were the case we should also make the flags exclusive by putting them in a group. I don't know which one is better though. What do you think?
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.
I can go either way. Both can have confusing situations and there is consensus on q/v existing but not how to handle multiple logging levels.
btw here is my python code I've been using for 5-10 years
# We want to default to WARNING
# Verbosity gives us granularity to control past that
if 0 < args.verbose and 0 < args.quiet:
parser.error("Mixing --verbose and --quiet is contradictory")
verbosity = 2 + args.quiet - args.verbose
verbosity = max(verbosity, 0)
verbosity = min(verbosity, 4)
args.logging_level = {
0: logging.DEBUG,
1: logging.INFO,
2: logging.WARNING,
3: logging.ERROR,
4: logging.CRITICAL,
}[verbosity]
0c3c86c
to
eb4bfae
Compare
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.
Okay, this does two things (in one commit…). Let me comment on each one because I feel like we should discuss this before we make these quite essential changes.
-q
. I agree it's a good idea – but I'd very much expect this to conflict with passing-v
!- Default to "info" level. This is surprising. I can see reasons for "warn", but "info"? Which app in the wild defaults to info?
Both together: If you were to default to warn, -q
could be used to only show errors. (The log crate doesn't have a "critical" level.)
If people are using INFO as user-facing information messages, e.g. in place of An example of me using INFO: https://github.com/cobalt-org/cobalt.rs/blob/master/src/bin/cobalt/serve.rs#L136 |
I have a PR that at least makes the log level configurable See #18 |
Defaults to log level info and adds a
--quiet
flag. Closes #10. Closes #9. Thanks!