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 failure to anyhow #59

Merged
merged 1 commit into from
Aug 12, 2022
Merged

Switch from failure to anyhow #59

merged 1 commit into from
Aug 12, 2022

Conversation

adumbidiot
Copy link
Contributor

This PR switches the error type of this CLI from failure to anyhow since failure is deprecated now.

@adumbidiot
Copy link
Contributor Author

CI failure is due to rust-lang/cargo#9788 I believe

Copy link
Collaborator

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

👍 awesome to see this cuts down on the number of dependencies! If you rebase the PR so CI reruns I'm happy to merge it :)

@@ -218,7 +218,7 @@ fn main() {
debug!("Writing timestamp file in: {:?}", path);
match Timestamp::new().store(path.as_path()) {
Ok(_) => {}
Err(e) => error!("Failed to write timestamp file: {}", e),
Err(e) => error!("Failed to write timestamp file: {:?}", e),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

anyhow has, to my knowledge, 2 different error output options. One just prints a simple one-line error message, which is usually just the error context. The other is more detailed and prints the entire error chain. Here is an example of the second option, taken from anyhow's docs:

Error: Failed to read instrs from ./path/to/instrs.json

Caused by:
    No such file or directory (os error 2)

I opted for the second option as it is more detailed and more helpful if one runs into an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah perfect - for some reason I thought it was {:#} but the docs say it's {:?} like you have here: https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations

@jyn514 jyn514 merged commit 842a8c9 into holmgr:master Aug 12, 2022
@adumbidiot adumbidiot deleted the anyhow branch August 12, 2022 23:50
@marcospb19 marcospb19 mentioned this pull request Oct 5, 2023
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