-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Use the "cargo metadata" command instead of manipulating paths directly #828
Use the "cargo metadata" command instead of manipulating paths directly #828
Conversation
Thank you, @Michael-F-Bryan! Looks reasonable. I'll take a closer look on Monday. |
I found a second issue which is related to the assumptions we make about paths and directory structures. When my projects all have a common prefix (e.g. My second commit switches the naming logic to use a package's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the pull request, @Michael-F-Bryan!
I've left some comments about the error handling. Feel free to address this in a follow-up commit, if you want. If not, I will open an issue to track it shortly.
let metadata = cargo_metadata::MetadataCommand::new() | ||
.current_dir(&crate_dir) | ||
.exec() | ||
.map_err(metadata_error_to_io)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this conversion. The function doesn't need to return io::Error
. It can return a custom error type that covers all its failure modes. (The crate already depends on thiserror
, so we don't even need an additional dependency.)
|
||
let msg = format!("\"{}\" doesn't point to a crate", dir.display()); | ||
|
||
Err(io::Error::new(io::ErrorKind::Other, msg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use a more appropriate custom error type here. We can probably reuse whatever Model::from_path
will end up returning.
727ab81
to
9baa869
Compare
No description provided.