-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: implement autocli customization #13251
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #13251 +/- ##
==========================================
+ Coverage 53.89% 53.90% +0.01%
==========================================
Files 643 643
Lines 55464 55464
==========================================
+ Hits 29891 29897 +6
+ Misses 23173 23168 -5
+ Partials 2400 2399 -1
|
Anyone able to review this? |
for _, cmd := range customCmds { | ||
queryCmd.AddCommand(cmd) | ||
} |
Check failure
Code scanning / gosec
the value in the range statement should be _ unless copying a map: want: for key := range m
for name := range commandOptions.FlagOptions { | ||
if fields.ByName(protoreflect.Name(name)) == nil { | ||
return nil, fmt.Errorf("can't find field %s on %s specified as a flag", name, messageType.Descriptor().FullName()) | ||
} | ||
} |
Check failure
Code scanning / gosec
the value in the range statement should be _ unless copying a map: want: for key := range m
for moduleName, modOpts := range moduleOptions { | ||
if customCmds[moduleName] != nil { | ||
// custom commands get added lower down | ||
continue | ||
} | ||
|
||
queryCmdDesc := modOpts.Query | ||
if queryCmdDesc != nil { | ||
cmd, err := b.BuildModuleQueryCommand(moduleName, queryCmdDesc) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
queryCmd.AddCommand(cmd) | ||
} | ||
} |
Check failure
Code scanning / gosec
the value in the range statement should be _ unless copying a map: want: for key := range m
list.Append(protoreflect.ValueOfInt64(int64(x))) | ||
} | ||
}) | ||
return newListValue(val, func(x uint) protoreflect.Value { return protoreflect.ValueOfUint64(uint64(x)) }) |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
I'm taking a look at this, sorry for the delay |
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! I carefully reviewed the client/v2/cli package and its tests. Also understood the general gist of the flag subpackage, but didn't do a thorough line-by-line review of that package. Good to get merged 👍 pending the small nits/questions below
Addressed all review comments. Okay to merge this? |
What to do about these gosec warnings? I fail to see how any of them are serious issues |
@tac0turtle I think I actually need your ACK here for some reason: |
seems tests in the cli/v2 are failing |
It was a glitch. So strange |
Description
Ref #11775
This follows up on #11763 and implements the customization options specified there for query commands in
client/v2/cli
.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change