Skip to content

Conversation

@clumens
Copy link
Contributor

@clumens clumens commented Nov 5, 2025

argparse will set metavar to the value of dest= by default. If dest= is not explictly set, then that value defaults to the same name as the first long option string. Thus in a lot of cases, we don't need to specify metavar.

argparse will set metavar to the value of dest= by default.  If dest= is
not explictly set, then that value defaults to the same name as the
first long option string.  Thus in a lot of cases, we don't need to
specify metavar.
@clumens clumens requested a review from nrwahl2 November 5, 2025 16:28
@clumens
Copy link
Contributor Author

clumens commented Nov 5, 2025

Yes this is really nit-picky, but it came up while I was reviewing #3982 and less code is almost always better.


parser.add_argument("-t", metavar="SUBAGENT", dest="subagent",
nargs=1, default="none", help="sub-agent")
parser.add_argument("-t", dest="subagent", nargs=1, default="none",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure nargs=1 could go away in all these too. You'd just need to treat the stored value as a string instead of as a list. Which would just mean dropping the normalize_options() function altogether. This can go in a later PR.

@nrwahl2 nrwahl2 merged commit fcb4e67 into ClusterLabs:main Nov 6, 2025
1 check passed
@clumens clumens deleted the unnecessary-metavar branch November 17, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants