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.

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

Move to clap derive #15

Merged
merged 13 commits into from
Oct 23, 2024
Merged

Conversation

eserilev
Copy link
Contributor

Move from clap builder to clap derive. Clap derive is a bit nicer to use because you can define the flag types alongside the flag definitions. This removes the need to use the cli helpers parse_optional and parse_required.

@jking-aus jking-aus changed the base branch from stable to unstable October 23, 2024 00:32
@jking-aus
Copy link
Contributor

hey man, thanks for this - would you mind addressing the conflicts? was based on stable but should be unstable

@eserilev
Copy link
Contributor Author

sorry about that, conflicts should be resolved now

@AgeManning AgeManning changed the base branch from unstable to stable October 23, 2024 01:16
@AgeManning AgeManning changed the base branch from stable to unstable October 23, 2024 01:16
Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

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

Oh yeah this is cool!

Thanks @eserilev!

// TODO define secrets_dir flag
// if cli_args.secrets_dir.is_some() {
// secrets_dir = cli_args.secrets_dir;
// }
Copy link
Member

Choose a reason for hiding this comment

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

Still need to add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yep, its now added

@AgeManning AgeManning self-requested a review October 23, 2024 23:06
@AgeManning AgeManning merged commit 7c2274a into sigp:unstable Oct 23, 2024
7 checks passed
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.

3 participants