Conversation
|
Compatible with rust-bitcoin/rust-bitcoin#603 |
sgeisler
left a comment
There was a problem hiding this comment.
Looks pretty good, much simpler than the other one 😅 One question that came to my mind: do we really need alloc for everything we do with core2 or could that even be a separate feature? At least our hash impls should not allocate imo.
|
Yes, it's definitely possible to have an alloc feature that is additive to the core2 feature. |
|
I'll followup on the alloc stuff soon |
|
OK, I added the alloc feature |
Cargo.toml
Outdated
| std = [] | ||
| # The no-std feature enables core2 and the alloc feature adds the alloc crate to that. | ||
| # You can still just disable std by disabling default features, without enabling these two. | ||
| no-std = ["core2"] |
There was a problem hiding this comment.
Do we need this as a feature? Users should be able to specify the core2 feature without this.
There was a problem hiding this comment.
removed, although I'm not sure how to best communicate to the developer what are the main features they should be interested in. I expanded the comment.
There was a problem hiding this comment.
I'm not sure how to communicate features at all - they aren't really mentioned in anything, and just using Cargo.toml doesn't really solve it. Ideally we'd put it in the top-line lib.rs docs.
TheBlueMatt
left a comment
There was a problem hiding this comment.
LGTM, aside from dropping the no-std feature.
sgeisler
left a comment
There was a problem hiding this comment.
Looks good. Could use some squashing, at least the little alloc fixup.
|
ugh, Rust 1.29 |
|
I think there was a trick for that, was it importing |
|
OK, maybe this will do it |
TheBlueMatt
left a comment
There was a problem hiding this comment.
LGTM. I think maybe we should start documenting all available features in top lib.rs comments, but that isn't specific to this PR, just something I think all our crates should do.
|
squashed |
Direct core2 support.
For context, see discussions in #127
Note that the
no-stdfeature enables core2 and alloc, but you can still compile without eitherstdorno-std, to eliminate the alloc dependency.