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

Add tutorial for how to use EyreContext #8

Closed
wants to merge 4 commits into from
Closed

Add tutorial for how to use EyreContext #8

wants to merge 4 commits into from

Conversation

yaahc
Copy link
Collaborator

@yaahc yaahc commented Apr 12, 2020

No description provided.

Comment on lines +30 to +34
//! ```rust
//! fn default(_: &(dyn std::error::Error + 'static)) -> Self {
//! Self
//! }
//! ```
Copy link

Choose a reason for hiding this comment

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

Might be worth including the impl EyreContext block here, so it's clear where in the code this default method should be placed.
I intuited that this was supposed to be a Default::default, but it's not.

Comment on lines +7 to +11
//! * Start with defining an empty context that captures nothing
//! * Add support for capturing backtraces
//! * skip backtrace capture if our source captured one
//! * Add support for a http status codes
//! * Implement a setter for Result<T, Report> with a new Trait
Copy link

Choose a reason for hiding this comment

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

It'd be nice to have anchor links here for the TOC, to jump to the relevant sections.

Comment on lines +36 to +37
//! And next we need to implement `debug` to tell eyre how to format the final report including
//! your custom context:
Copy link

Choose a reason for hiding this comment

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

I think it'd be good to clarify why this method is called "debug", and how it differs from fmt::Debug, perhaps with an intra-doc link to the documentation on the EyreContext trait.

Comment on lines +62 to +66
//! Things to note, we're using some utilities provided by `eyre` for writing error reports,
//!
//! - `Chain`: iterator for errors while the `chain` fn on the error trait is still unstable.
//! - `Indenter`: a wrapper for formatters that inserts indentation after newlines for handling
//! multi line error messages nicely
Copy link

Choose a reason for hiding this comment

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

This is a bit of an awkward place to mention the crates being used. I think it could perhaps be nice to mention them off-hand at the beginning, and have more detailed information in an appendix-type thing?

//!
//! # Introduction
//!
//! In this tutorial we will go through the process of implementing a custom context step by step:
Copy link

Choose a reason for hiding this comment

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

I think it'd be really valuable to have a section before you begin implementing a custom context explaining what a custom context is, and why you might want one. It doesn't necessarily have to be here, but a sentence mentioning where in the docs you can learn the motivation would be nice.

//! // ...
//! }
//! ```
//!
Copy link

Choose a reason for hiding this comment

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

I think including some example code here which reads the status from an error as well would be awesome!

Comment on lines +154 to +156
//! if f.alternate() {
//! return core::fmt::Debug::fmt(error, f);
//! }
Copy link

Choose a reason for hiding this comment

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

It'd be good to mention documentation which explains why alternate is being checked here.

@yaahc yaahc closed this Jun 22, 2020
pksunkara pushed a commit that referenced this pull request Sep 20, 2023
@yaahc yaahc deleted the tutorial branch March 14, 2024 22:18
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