refactor(cli): migrate from docopt-go to go-arg (#965)#980
refactor(cli): migrate from docopt-go to go-arg (#965)#980zeroshade merged 2 commits intoapache:mainfrom
Conversation
Replace flat Config struct and boolean dispatch with typed subcommand structs. Each command owns its flags, subcommand help is auto-generated, and adding new commands is self-contained.
There was a problem hiding this comment.
Thanks for the migration, moving off docopt looks like the right direction.
I looked at #696 as related context, and I would not fold the broader dependency-tree work into this PR. The general direction there seems to be package isolation / registration patterns rather than a multi-module split, and the real dependency cost is mostly around cloud SDK packages, not this CLI parser. So I’d keep this PR focused on the parser swap.
A few things I’d ask to fix before merge:
- run
go mod tidy - fix missing-subcommand handling so cases like
iceberg compact TBLshow useful help instead ofinvalid subcommand: TBL - resolve the
describepositional ambiguity, preferably by keepingIDENTIFIERrequired - restore the CDC guidance in
--preserve-dead-equality-deleteshelp text - add a small table-driven parser test for the documented command forms
With those fixed, this looks good to me.
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.55.0 // indirect | ||
| github.com/Microsoft/go-winio v0.6.2 // indirect | ||
| github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect | ||
| github.com/alexflint/go-arg v1.6.1 // indirect |
There was a problem hiding this comment.
is this tidied?
my local go mod tidy has some diff.
Tidy go.mod (promote go-arg to direct, remove docopt-go), fix missing-subcommand handling to show full help, make describe IDENTIFIER required, restore CDC guidance in --preserve-dead-equality-deletes, and add table-driven parser test.
|
Thanks for the review! All five items addressed: tidied go.mod (moved go-arg to direct, removed docopt-go), missing-subcommand cases like iceberg compact now print full help with available commands, describe requires IDENTIFIER positionally, restored the CDC guidance in --preserve-dead-equality-deletes help text, and added a table-driven parser test covering all documented command forms. |
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
As this grows, we might want to move some of the Cmd definitions into their own files and split more of the functionality to make things more readable. but this is fine for now. Thanks for this!
3 participants
Replace flat Config struct and boolean dispatch with typed subcommand structs. Each command owns its flags, subcommand help is auto-generated, and adding new commands is self-contained.
Closes: #965