libbpf-tools: fix some tools error info and usage #3245
Conversation
Signed-off-by: Wenbo Zhang <ethercflow@gmail.com>
ethercflow
requested review from
4ast,
brendangregg,
drzaeus77,
goldshtn and
yonghong-song
as code owners
There was a problem hiding this comment.
There are a bunch more cases of arbitrary changing the case of existing command line arguments with no justification. Please provide rationale for the changes in your commit messages, not just the "what".
Also, while I have your attention. Can you please check #3224 (comment)? Would you be able to add that check? Thanks!
| "\n" | ||
| "EXAMPLES:\n" | ||
| " biolatency # summarize block I/O latency as a histogram\n" | ||
| " biolatency 1 10 # print 1 second summaries, 10 times\n" | ||
| " biolatency -mT 1 # 1s summaries, milliseconds, and timestamps\n" | ||
| " biolatency -MT 1 # 1s summaries, milliseconds, and timestamps\n" |
There was a problem hiding this comment.
biolatency.py is using -m, why sudden and diverging change?
There was a problem hiding this comment.
There are a bunch more cases of arbitrary changing the case of existing command line arguments with no justification. Please provide rationale for the changes in your commit messages, not just the "what".
Oh, my motivation is I think maybe it's easier to remember. Lowercase parameters need parameter values (except -v), and uppercase ones belong to the switch class, so you don't need to add parameter values.
Also, while I have your attention. Can you please check #3224 (comment)? Would you be able to add that check? Thanks!
Oh sorry, I haven't checked the email from iovisor/bcc recently, I missed it. I will look at it later （Maybe weekend）.
There was a problem hiding this comment.
I haven't heard about such naming convention, tbh. I think the harm of changing existing arguments and, especially, diverging from .py tool, is much bigger. So let's not do such renamings unnecessarily.
And thanks for checking the other PR.
There was a problem hiding this comment.
Got it, I will be consistent with .py tool. Thanks!
| "\n" | ||
| "EXAMPLES:\n" | ||
| " cpudist # summarize on-CPU time as a histogram" | ||
| " cpudist -O # summarize off-CPU time as a histogram" | ||
| " cpudist 1 10 # print 1 second summaries, 10 times" | ||
| " cpudist -mT 1 # 1s summaries, milliseconds, and timestamps" | ||
| " cpudist -MT 1 # 1s summaries, milliseconds, and timestamps" |
There was a problem hiding this comment.
similarly. py version is sticking with -m. What's your rationale for making these incompatible changes?
|
|
||
| static const struct argp_option opts[] = { | ||
| { "duration", 'd', "DURATION", 0, "Total duration of trace in seconds" }, | ||
| { "extended", 'e', NULL, 0, "Extended fields output" }, | ||
| { "extended", 'E', NULL, 0, "Extended fields output" }, |
There was a problem hiding this comment.
this seems to be libbpf-tools exclusive argument, but again, why changing it in the first place?
Signed-off-by: Wenbo Zhang ethercflow@gmail.com