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

Make rand and rustc-serialize dependencies optional. #154

Merged
merged 1 commit into from
Jan 8, 2016
Merged

Make rand and rustc-serialize dependencies optional. #154

merged 1 commit into from
Jan 8, 2016

Conversation

briansmith
Copy link
Contributor

Previously, the rand and rustc-serialize dependencies were optional
except they were required for the bigint feature.

Make the dependency on the rand crate optional in all cases.
including when the bigint feature is selected. Some of the tests for
the bigint feature are randomized so, while rand is now an optional
dependency, it is a non-optional dev-dependency.

Similarly, make the dependency on the rustc-serialize crate optional
in all cases, including when the bigint feature is selected.

Previously, the `rand` and `rustc-serialize` dependencies were optional
except they were required for the `bigint` feature.

Make the dependency on the `rand` crate optional in all cases.
including when the `bigint` feature is selected. Some of the tests for
the bigint feature are randomized so, while `rand` is now an optional
dependency, it is a non-optional dev-dependency.

Similarly, make the dependency on the `rustc-serialize` crate optional
in all cases, including when the `bigint` feature is selected.
@briansmith
Copy link
Contributor Author

The Nightly build fails because of rust-lang/rust#30713, which should be fixed in the next Rust Nightly. With this commit, packages depending on num can avoid that breakage by using default-features = false and specifying features = [<anything but "rand">] in the dependencies.

@cuviper
Copy link
Member

cuviper commented Jan 8, 2016

I think this is good. My only concern is that someone using just feature bigint will now lose functionality, so that's a breaking change. But since this isn't the default case, I'm leaning toward accepting that. What do you think @hauleth?

@hauleth
Copy link
Member

hauleth commented Jan 8, 2016

I don't see how one could loose functionality by using only bignum. Rather I can see how one could loose functionality when providing no-default and forgetting to add rand to the feature list. So yes. I would consider it somewhat breaking change, but I do not think that impact will be wide.

@cuviper
Copy link
Member

cuviper commented Jan 8, 2016

OK, you described what I meant. We'll go ahead then.

@homu r+ 9e3e855

@homu
Copy link
Contributor

homu commented Jan 8, 2016

⌛ Testing commit 9e3e855 with merge dbcc359...

homu added a commit that referenced this pull request Jan 8, 2016
Make `rand` and `rustc-serialize` dependencies optional.

Previously, the `rand` and `rustc-serialize` dependencies were optional
except they were required for the `bigint` feature.

Make the dependency on the `rand` crate optional in all cases.
including when the `bigint` feature is selected. Some of the tests for
the bigint feature are randomized so, while `rand` is now an optional
dependency, it is a non-optional dev-dependency.

Similarly, make the dependency on the `rustc-serialize` crate optional
in all cases, including when the `bigint` feature is selected.
@homu
Copy link
Contributor

homu commented Jan 8, 2016

💔 Test failed - status

@cuviper
Copy link
Member

cuviper commented Jan 8, 2016

Grr, still no new published nightly to fix rand.
Locally with rustc 1.7.0-nightly (8f11a9ef4 2016-01-03) looks fine.

cuviper added a commit that referenced this pull request Jan 8, 2016
Make `rand` and `rustc-serialize` dependencies optional.
@cuviper cuviper merged commit 9ec4106 into rust-num:master Jan 8, 2016
@hauleth
Copy link
Member

hauleth commented Jan 8, 2016

Yeah, I am also waiting for that fix. Today should be new nightly build. It is strange.

Łukasz Jan Niemier

Dnia 8 sty 2016 o godz. 18:52 Josh Stone notifications@github.com napisał(a):

Grr, still no new published nightly to fix rand.
Locally with rustc 1.7.0-nightly (8f11a9ef4 2016-01-03) looks fine.


Reply to this email directly or view it on GitHub.

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