Make rand dependency optional
#483
Conversation
Most users probably don't use the rand impls, so the `rand` crate pulls a large number of dependencies into the dependency tree which is just wasted compilation time.
|
Looks fine by me overall. Leaving for others to have a look as well. |
No, currently not. We probably would like to have a test case without any optional features (without |
|
Just doing |
|
@kvark I just pushed a commit. I hope this is what you had in mind. The The two nightly failures don't have anything to do with this change. |
As |
|
@LukasKalbertodt thank you for the link! Upon reading it carefully, I don't think it applies to our case. It's more about what features of dependent projects our project decides to enable. For example, we depend on |
|
@kvark Might be true 🤔 I guess you don't plan on releasing a major new version of And regarding this PR: can this be merged regardless of its breaking-nature? Or should I create a new PR against a different branch? |
|
@ozkriff @tomaka @brendanzab @csherratt @torkleyy I was about to say that I'm not intimately familiar with cgmath workflow and release procedures, and that we should ask somebody else, but then I scanned the history of merged PRs and mostly saw myself lately :( yikes! I hope I'm not the last holdout of Anyhow, in other projects I have, the @LukasKalbertodt typically, releasing a breaking version upon seeing the first breaking change would waste a lot of effort by the community in updating... (unless they rely on mint). So what I do is - scanning the list of issues and looking for other breaking changes that we accepted and haven't got merged/implemented yet. This is work, and I don't expect you to follow-up on it, but that's the process that I find working best. For this PR, since we are breaking the API anyway, let's not enable |
|
@kvark Sounds reasonable. I pushed a commit removing In general, I could see myself helping out to work on and co-maintain |
|
bors r+ |
483: Make `rand` dependency optional (but enabled by default) r=kvark a=LukasKalbertodt Closes #481 This PR leaves the feature enabled by default so that this can be released as a minor version soon. (I would suggest to disable it by default in future versions, but that's another discussion.) The changes are pretty straight forward with one catch: I changed some `use` statements to nested imports (otherwise, there would be even more `#[cfg(feature = "rand")]` lines). Nested imports were stabilized in 1.25 (see [the edition guide on this feature](https://doc.rust-lang.org/edition-guide/rust-2018/module-system/nested-imports-with-use.html)). I'm not sure how cgmaths policy on minimum compiler version is. Maybe cgmath already requires a >= 1.25 compiler for other reasons. If my change is a problem, just tell me and I will change the imports back. Co-authored-by: Lukas Kalbertodt <lukas.kalbertodt@gmail.com>
Build succeeded |
kvark
changed the title
rand dependency optional (but enabled by default)rand dependency optional
Closes #481
This PR leaves the feature enabled by default so that this can be released as a minor version soon. (I would suggest to disable it by default in future versions, but that's another discussion.)
The changes are pretty straight forward with one catch: I changed some
usestatements to nested imports (otherwise, there would be even more#[cfg(feature = "rand")]lines). Nested imports were stabilized in 1.25 (see the edition guide on this feature). I'm not sure how cgmaths policy on minimum compiler version is. Maybe cgmath already requires a >= 1.25 compiler for other reasons. If my change is a problem, just tell me and I will change the imports back.