Skip to content

error: enable core::error::Error for all error payloads #693

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 12, 2023
Merged

error: enable core::error::Error for all error payloads #693

merged 2 commits into from
Mar 12, 2023

Conversation

phip1611
Copy link
Member

Implements a solution for the discussion in #691.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 self-assigned this Mar 12, 2023
@phip1611 phip1611 linked an issue Mar 12, 2023 that may be closed by this pull request
Error trait for type Error<Data> where Data is not Display #691
Closed
@phip1611 phip1611 changed the title Error display error: enable core::error::Error for all error payloads Mar 12, 2023
nicholasbishop
nicholasbishop approved these changes Mar 12, 2023
View reviewed changes
Copy link
Member

@nicholasbishop nicholasbishop left a comment

Choose a reason for hiding this comment

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

This seems like a good solution to me. Should probably mention in the changelog that Error's Data parameter is no longer required to implement Display.

@phip1611
Copy link
Member Author

phip1611 commented Mar 12, 2023 •
edited
Loading

I've added another commit because I think the current doc comment doesn't reflect the usage
well. What do you think?

@phip1611 phip1611 requested a review from nicholasbishop March 12, 2023 14:47
nicholasbishop
nicholasbishop approved these changes Mar 12, 2023
View reviewed changes
Copy link
Member

@nicholasbishop nicholasbishop left a comment

Choose a reason for hiding this comment

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

lgtm

@phip1611 phip1611 merged commit 905c176 into rust-osdev:main Mar 12, 2023
@phip1611 phip1611 deleted the error-display branch March 12, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicholasbishop nicholasbishop nicholasbishop approved these changes

Assignees

@phip1611 phip1611

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error trait for type Error<Data> where Data is not Display
2 participants
@phip1611 @nicholasbishop