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

chore: Expose error creation methods #224

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

ChaoticTempest
Copy link
Member

addresses #194.

Thought it would be nice to have these be exposed since the errors are in good shape to be used at higher levels if users want to make use of them to build their own errors. Also added the Other variant to ErrorKind since users can have different categories of errors other than the ones defined for their higher level workspaces libraries. Any objections to adding this?

/// Construct a workspaces [`Error`] with the full details of an error which includes
/// the internal error it references, the custom error message with further context
/// and the [`ErrorKind`] that represents the category of error.
pub fn full<T, E>(kind: ErrorKind, msg: T, error: E) -> Self
Copy link
Contributor

Choose a reason for hiding this comment

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

Just going to share some low-confidence opinions that having the semantics of error and message separate seems unnecessarily complex and having this interface now be public seems like it might be a pain to maintain in the future.

If you're confident this is useful and these should all be exposed, go for making this change, I just noticed that there is no actual usage of the downcasting through into_inner of Error and exposing all of these variants will make it even harder to switch later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, how about just expose only custom/message errors then. There's only a couple cases where full is useful in workspaces itself where we just provide some extra context about the error. So let's not expose it for now until there's a community need

@ChaoticTempest ChaoticTempest force-pushed the chore/expose-error-creation branch from 52b2a0d to 92c1ee1 Compare November 4, 2022 22:36
@ChaoticTempest ChaoticTempest merged commit 48a5cd7 into main Nov 17, 2022
@ChaoticTempest ChaoticTempest deleted the chore/expose-error-creation branch November 17, 2022 20:25
@frol frol mentioned this pull request Oct 4, 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