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

Switch from structopt to clap #446

Merged
merged 1 commit into from
May 6, 2022
Merged

Switch from structopt to clap #446

merged 1 commit into from
May 6, 2022

Conversation

lann
Copy link
Collaborator

@lann lann commented May 5, 2022

No description provided.

Copy link
Contributor

@fibonacci1729 fibonacci1729 left a comment

Choose a reason for hiding this comment

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

SUPERB. LGTM

src/bin/spin.rs Show resolved Hide resolved
@Mossaka
Copy link
Contributor

Mossaka commented May 5, 2022

Not intended to review the code, but I have a question: what's the motivation to change structopt to clap. I know little about them, and they seem to have the effects to me.

@lann
Copy link
Collaborator Author

lann commented May 5, 2022

Not intended to review the code, but I have a question: what's the motivation to change structopt to clap. I know little about them, and they seem to have the effects to me.

It is essentially just an upgrade. The structopt crate was deprecated and its functionality was merged into clap v3. The specific reason to upgrade now is that clap has a new feature that will help with #444

Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Good times

@@ -1,33 +1,32 @@
use std::path::PathBuf;

use anyhow::Result;
use clap::Parser;

Copy link
Member

Choose a reason for hiding this comment

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

Official request for bikeshedding (that should be ignored in this PR).

Is there a convention on order imports that we want to follow?

@lann lann merged commit f267af0 into fermyon:main May 6, 2022
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.

6 participants