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 no_std feature, impl std::error::Error #22

Merged
merged 2 commits into from
Oct 21, 2016

Conversation

abonander
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 97.033% when pulling 16da533 on abonander:master into 49bf4d8 on seanmonstar:master.

@seanmonstar seanmonstar merged commit cdcc850 into seanmonstar:master Oct 21, 2016
@abonander
Copy link
Contributor Author

@seanmonstar Are you waiting to publish a new version or did you forget again? ;P

@seanmonstar
Copy link
Owner

Something tickles me weird about having the no_std be a feature. I wanted to think about it some to see if there's something better.

@abonander
Copy link
Contributor Author

It's a common convention, I made it a default so it didn't break any existing usage expecting it to work with core right out of the box.

@abonander
Copy link
Contributor Author

@seanmonstar Would you prefer to leave the #![no_std] unconditional and just have a std feature? I think that would work:

#![no_std] 

#[cfg(feature = "std")]
extern crate std;

#[cfg(feature = "std")]
impl ::std::error::Error for Error { /* ... */ }

@seanmonstar
Copy link
Owner

Ignore me, it's perfect. Thanks!

@jethrogb
Copy link
Contributor

jethrogb commented Nov 6, 2016

The convention is to have a feature named std, see this comment for rationale.

Using this convention is not a breaking change, since no httparse versions that use no_std have been published yet.

@seanmonstar
Copy link
Owner

@jethrogb so, have this instead:

[features]
default = ["std"]
std = []

And anyone wanting to use it in a no_std environment sets default-features = false.

@seanmonstar
Copy link
Owner

If that seems correct, I've made a PR to do that, and I can release the new version today #23

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.

4 participants