-
Notifications
You must be signed in to change notification settings - Fork 11.9k
[ISSUE #6108] Optimize command line tools QueryMsgByKey& QueryMsgTraceById #6115
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
…ber of data bars can be specified
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.
Hi @socutes CI fails and you can import codestyle into your idea. reference: https://rocketmq.apache.org/docs/contributionGuide/02code-guidelines
Codecov Report
@@ Coverage Diff @@
## develop #6115 +/- ##
=============================================
- Coverage 43.18% 43.17% -0.02%
+ Complexity 8852 8847 -5
=============================================
Files 1094 1094
Lines 77153 77189 +36
Branches 10063 10069 +6
=============================================
+ Hits 33321 33325 +4
- Misses 39665 39690 +25
- Partials 4167 4174 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@zhouxinyu PTAL |
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
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.
Hi, thanks your contributions, you should also update the operation.md
docs.
opt.setRequired(false); | ||
options.addOption(opt); | ||
|
||
opt = new Option("num", "maxNum", true, "The maximum number of messages returned by the query, default:64"); |
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.
How about change the option name to c
(messageNumber), like the ConsumeMessageCommand
. Simple unified use of single letter.
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 was thinking about this before, and I didn't think of the right shorthand, so I used num.
Your suggestion is very good. I'll revise it.
#5772, Maybe this Pr will close. |
hi, I didn't notice this closed issue. This problem has not been fundamentally solved. The main branch does not have this function. |
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~
close #6108