Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ArgsManager: support subcommand-specific options #28802

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

ArgsManager: support subcommand-specific options #28802

wants to merge 3 commits into from
+129 −45

Conversation

ajtowns
Copy link
Contributor

@ajtowns ajtowns commented Nov 6, 2023

Adds the ability to link particular options to one or more subcommands, and uses this feature in bitcoin-wallet. Separates out the help information for subcommand-specific options (duplicating it if an option applies to multiple subcommands), and provides a function for checking if some options have been specified that only apply to different subcommands.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 6, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/28802.

Reviews

See the guideline for information on the review process.

Type Reviewers
Approach ACK stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29838 (Feature: Use different datadirs for different signets by BrandonOdiwuor)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ajtowns
Copy link
Contributor Author

ajtowns commented Nov 6, 2023 •
edited
Loading

Implements this comment from when these commands were introduced originally. Main motivation is that I'd like to add a new subcommand to bitcoin-util unrelated to the existing grind functionality, and have options for it.

@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from d335f7f to abba7c5 Compare November 6, 2023 09:08
@DrahtBot DrahtBot mentioned this pull request Nov 6, 2023
Draft
@maflcko
Copy link
Member

maflcko commented Nov 6, 2023

tool_wallet.py fails CI

@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from abba7c5 to 162c5ab Compare November 6, 2023 18:37
@DrahtBot DrahtBot removed the CI failed label Nov 6, 2023
achow101
achow101 reviewed Nov 6, 2023
View reviewed changes
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 162c5ab to 86fab48 Compare November 6, 2023 22:50
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 86fab48 to 6be2b82 Compare November 6, 2023 23:02
@DrahtBot DrahtBot removed the CI failed label Nov 6, 2023
stickies-v
stickies-v reviewed Nov 8, 2023
View reviewed changes
Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

Approach ACK

@ajtowns ajtowns mentioned this pull request Nov 8, 2023
Closed
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch 2 times, most recently from d5e680e to 1cd91d4 Compare December 5, 2023 14:41
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 1cd91d4 to 15875ea Compare December 5, 2023 20:57
@DrahtBot DrahtBot mentioned this pull request Dec 16, 2023
Merged
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 15875ea to 905ed6b Compare February 13, 2024 07:10
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 905ed6b to d10e294 Compare March 12, 2024 23:56
@DrahtBot DrahtBot mentioned this pull request Apr 9, 2024
Open
@ajtowns ajtowns requested review from stickies-v and achow101 April 10, 2024 09:21
@ajtowns ajtowns mentioned this pull request May 8, 2024
Merged
@DrahtBot DrahtBot mentioned this pull request May 31, 2024
Closed
@DrahtBot DrahtBot mentioned this pull request Jun 10, 2024
Merged
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from d10e294 to 793d116 Compare June 17, 2024 13:11
@DrahtBot DrahtBot mentioned this pull request Aug 26, 2024
Closed
@ajtowns ajtowns mentioned this pull request Sep 2, 2024
Merged
@ajtowns ajtowns mentioned this pull request Sep 5, 2024
Open
@ajtowns ajtowns force-pushed the 202311-argsman-subcmd-options branch from 793d116 to 80c0927 Compare September 6, 2024 00:20
This was referenced Nov 7, 2024
Open
Open
@DrahtBot
Copy link
Contributor

Needs rebase, if still relevant

@ajtowns ajtowns marked this pull request as draft March 16, 2025 05:58
@ajtowns
Copy link
Contributor Author

ajtowns commented Mar 16, 2025

Leaving as draft pending #31250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanofsky ryanofsky Awaiting requested review from ryanofsky

@maflcko maflcko Awaiting requested review from maflcko

@stickies-v stickies-v Awaiting requested review from stickies-v

@achow101 achow101 Awaiting requested review from achow101

Assignees
No one assigned
Labels
CI failed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ajtowns @DrahtBot @maflcko @achow101 @stickies-v