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

improv: global modeler error type, finer-grained errors, using thiserror #5

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

flxzt
Copy link
Owner

@flxzt flxzt commented Jun 7, 2024

This refactors the errors into a separate module, restructures them a bit and introduces a crate-wide ModelerError intended to be returned in all results in the crate - currently it only holds the ElementError variant, but allows for future additions.
All error types are deriving the thiserror Error trait, improving their messages and general handling.

@flxzt flxzt requested a review from Doublonmousse June 7, 2024 08:39
Copy link
Collaborator

@Doublonmousse Doublonmousse left a comment

Choose a reason for hiding this comment

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

That looks good.
Having errors that can be nested like this is cool !

I think the description of the repository (the about) should be changed because the repository is not a binding anymore.

I'll have to update the rnote PR after this is merged to take into account the new error types

@flxzt
Copy link
Owner Author

flxzt commented Jun 7, 2024

Having errors that can be nested like this is cool !

Yeah and thiserror also automatically derives a From impl when a src field is annotated with #[from].

I think the description of the repository (the about) should be changed because the repository is not a binding anymore.

Done.

I also fixed some typos found throughout the crate

@flxzt flxzt merged commit 6e60b25 into main Jun 7, 2024
4 checks passed
@flxzt flxzt deleted the fz/modeler-errors branch June 7, 2024 11:31
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