Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

lib: port CLI handling to clap v3 #345

Merged
merged 2 commits into from
Sep 29, 2022
Merged

lib: port CLI handling to clap v3 #345

merged 2 commits into from
Sep 29, 2022

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Jul 19, 2022

This reworks CLI handling logic, porting to latest clap v3.

Closes: #373

@lucab
Copy link
Member

lucab commented Sep 28, 2022

I'm doing some other clap-v3 porting, so I can take this over too.

@lucab lucab force-pushed the clap-3 branch 2 times, most recently from 67268e9 to ebf97fd Compare September 28, 2022 10:26
@lucab
Copy link
Member

lucab commented Sep 28, 2022

For a start, I rebased the branch and brought it to green CI state.
There are tons of (deprecation) warnings right now though, I'd like to sort them out here.

cgwalters and others added 2 commits September 29, 2022 08:15
This reworks CLI handling logic, porting to latest clap v3.
@lucab lucab changed the title wip: clap 3 lib: port CLI handling to clap v3 Sep 29, 2022
@lucab lucab marked this pull request as ready for review September 29, 2022 09:07
@lucab
Copy link
Member

lucab commented Sep 29, 2022

Ok, porting here is completed and this is ready for review.
Please note that this is an API breaking change.
Also, clap v4 just got released and it is going to be another API break. We may want to consider switching to that before tagging a release here.

@cgwalters
Copy link
Member Author

Yeah in some conversation over at https://www.reddit.com/r/rust/comments/xqiqfy/clap_400_a_rust_argument_parser_is_released/ it seems like some people were aware a v4 was coming and held off porting from v2. But, I guess since we're doing this we might as well do v4?

Potentially related to this, we could split off the CLI code into a separate crate with its own semver churn. Definitely some cost/benefit to that.

Anyways...yeah I think I'd say we should go v4, but I haven't tried the port either myself. I can do it if you like.

@lucab
Copy link
Member

lucab commented Sep 29, 2022

@cgwalters when we land this, the clap-v4 update should be just a Cargo manifest bump (as this PR already gets rid of all the planned deprecations).

@cgwalters
Copy link
Member Author

LGTM as is but I can't approve this as it's my own PR so...feel free to approve 😄

Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM (feels like cheating) 😄

@lucab lucab merged commit 3e51d08 into ostreedev:main Sep 29, 2022
@cgwalters
Copy link
Member Author

Please note that this is an API breaking change.

Oh, right. Well, we forgot to bump semver for this. Urgh. Well...ok. In practice I can't imagine anything is using this API except rpm-ostree.

If something is, well, I can't imagine we're causing them that much pain.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli: port from structopt to clap-v3
2 participants