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

Error types everywhere instead of String #944

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

fluffysquirrels
Copy link

As discussed in issue #942.

@fluffysquirrels
Copy link
Author

Is there anything I can do to help move this along?

@Cobrand
Copy link
Member

Cobrand commented Jan 6, 2020

I'm sorry, I terribly misunderstood what you meant in the other issue, and unfortunately I think this PR is a breaking change only for a breaking change. Changing String to an enum with the only variant being String is a bit of a waste... If you added some variants, that would probably be worth the while, and that's what I thought you would do when we discussed in the other issue, but here, I don't think it's a necessary breaking change.

If you want your error type to implement Error, what we can do is create a tuple struct SdlErrorWrapper(String) which implements Error so it can be used by other libraries more easily... although this would only save a dozen lines at most, no more.

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