-
Notifications
You must be signed in to change notification settings - Fork 51
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
added no_std support #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I would not have guessed you were new to Rust.
I think FloatCore
will be enough that we can still have FromPrimitive
, but try it and let me know. There might be ways to get more of the tests working too, but that's less important.
Can you also add std
to ci/test_full.sh, for feature in ...
?
@@ -13,7 +13,7 @@ Add this to your `Cargo.toml`: | |||
|
|||
```toml | |||
[dependencies] | |||
num-rational = "0.1" | |||
num-rational = "0.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I hadn't updated this yet, since I'm not sure how long until 0.2 will be released, but I guess it's OK.
src/lib.rs
Outdated
if denom.is_zero() { | ||
Err(ParseRatioError { kind: RatioErrorKind::ZeroDenominator }) | ||
} else { | ||
Ok(Ratio::new(numer.clone(), denom.clone())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these clone
s are unnecessary now.
(probably weren't needed before either)
src/lib.rs
Outdated
@@ -955,6 +961,9 @@ impl FromPrimitive for Ratio<BigInt> { | |||
} | |||
} | |||
|
|||
// NOTE: this macro compiles just fine with no_std, it's hidden behind a feature since all of its | |||
// uses depend on the Float type, which depends on std. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think num_traits::float::FloatCore
will be enough here.
Sure enough, FloatCore worked like a charm. |
Thanks! I just tweaked a couple more things myself. bors r+ |
23: added no_std support r=cuviper a=robomancer-or Currently the `uom` crate (and who knows what other crates) cannot support no_std because it depends deeply on num-rational, so I'm adding no_std support here. Feedback definitely welcome, I'm brand new to Rust and I know I have a lot to learn.
Build succeeded |
Currently the
uom
crate (and who knows what other crates) cannot support no_std because it depends deeply on num-rational, so I'm adding no_std support here. Feedback definitely welcome, I'm brand new to Rust and I know I have a lot to learn.