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 owned variants for each error #65

Merged
merged 1 commit into from
Jul 28, 2017
Merged

Add owned variants for each error #65

merged 1 commit into from
Jul 28, 2017

Conversation

alexcrichton
Copy link
Contributor

For all fallible functions that consume Bytes a new InvalidFooShared error
has been added. Eventually this extra error type can retain the input Bytes
payload to be re-acquired on error if necessary.

Closes #48

Copy link
Collaborator

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

One question, do we want the suffix to be Bytes vs. Shared? I only went w/ a _shared suffix because _bytes was taken and eventually all those fns will be deprecated in favor of TryFrom impls?

Anyway, looks good to me.

@alexcrichton
Copy link
Contributor Author

I'm fine either way, @withoutboats or @seanmonstar, thoughts on naming here?

@seanmonstar
Copy link
Member

I don't really love the Shared part of the name, myself. I suppose Bytes suffix sounds better.

There is also the pattern from std, FromInvalidHeaderName, etc? Hm, that reads kinda odd...

  • InvalidHeaderNameShared
  • InvalidHeaderNameBytes
  • InvalidHeaderNameOwned
  • FromInvalidHeaderName
  • (InvalidHeaderName, Bytes)

For all fallible functions that consume `Bytes` a new `InvalidFooShared` error
has been added. Eventually this extra error type can retain the input `Bytes`
payload to be re-acquired on error if necessary.

Closes #48
@alexcrichton
Copy link
Contributor Author

Sure yeah a suffix of Bytes seems reasonable, updated

@alexcrichton
Copy link
Contributor Author

Ok I'm gonna merge for now, but we can of course tweak before release!

@alexcrichton alexcrichton merged commit 0f410fa into hyperium:master Jul 28, 2017
@alexcrichton alexcrichton deleted the owned-errors branch July 28, 2017 22:43
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.

3 participants