Skip to content

Conversation

@Logarithmus
Copy link

@Logarithmus Logarithmus commented Nov 11, 2021

Aims for saner defaults & to match cargo update behavior

@Logarithmus Logarithmus force-pushed the feat/cargo-workspace-by-default branch from 2ed18d7 to 26a2985 Compare November 11, 2021 00:51
Aims for saner defaults & to match `cargo update` behavior
@Logarithmus Logarithmus force-pushed the feat/cargo-workspace-by-default branch from 26a2985 to 28ee3f7 Compare November 11, 2021 14:37

/// Check if local manifest is virtual, i. e. corresponds to workspace root
pub fn is_virtual(&self) -> bool {
!self.data["workspace"].is_none()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The existence of a workspace table does not make it virtual. To be virtual, it also needs to not have a package.

Check out https://github.com/crate-ci/clap-cargo/blob/master/src/workspace.rs#L29 for a good approximation of what cargo does.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll fix it a bit later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively, we could just port the project over to clap-cargo.

Copy link
Author

Choose a reason for hiding this comment

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

clap-cargo look like an amazing project! I'm willing to help with the porting.

@Logarithmus Logarithmus requested a review from epage November 11, 2021 14:38
@Logarithmus Logarithmus mentioned this pull request Nov 15, 2021
@Logarithmus
Copy link
Author

Closing in favor of #537

@Logarithmus Logarithmus deleted the feat/cargo-workspace-by-default branch November 15, 2021 01:09
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