Skip to content

Mention that examples and doctests require std #439

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

Closed
wants to merge 1 commit into from

Conversation

vks
Copy link
Collaborator

@vks vks commented May 11, 2018

Also remove pointless attributes. (#![cfg(feature="std")] does not work
for binaries, because main will be missing.)

Also remove pointless attributes. (`#![cfg(feature="std")]` does not work
for binaries, because `main` will be missing.)
@dhardy
Copy link
Member

dhardy commented May 11, 2018

Are you sure it's pointless? I'm not really sure why cargo test --tests --no-default-features fails sometimes.

@vks
Copy link
Collaborator Author

vks commented May 11, 2018

Are you sure it's pointless?

You are right, the CI fails due to this patch. Not sure why. Maybe the cargo behavior is inconsistent across Rust versions?

@pitdicker
Copy link
Contributor

Can you think of another place to add the comment? I just removed that section from the readme in #437...

@pitdicker
Copy link
Contributor

The difference is that some travis configurations run

cargo test --features serde1,log,nightly,alloc

instead of the more complete:

cargo test --tests --features serde1,log,nightly,alloc

We could also use --lib and --examples if we want to be explicit.

@vks
Copy link
Collaborator Author

vks commented May 11, 2018

@dhardy
The CI fails on only two platforms and that was the only explanation I could think of. But according to the CI the cargo versions are the same. I don't understand those failures.

@pitdicker

Can you think of another place to add the comment? I just removed that section from the readme in #437...

I'm opposed to removing the testing section if we decide against making the examples and doctests compile without std. I probably will not be the last one to try to run cargo test --no-default-features. This is important to know for any contributor to Rand.

The difference is that some travis configurations run cargo test --features serde1,log,nightly,alloc instead of the more complete cargo test --tests --features serde1,log,nightly,alloc

Are you sure this is the reason? AFAIK, cargo test --features serde1,log,nightly,alloc does not disable std, so I don't see how this would be related to the CI failures.

@pitdicker
Copy link
Contributor

Are you sure this is the reason?

Yes, because one also tests the examples, the other not.

@pitdicker pitdicker mentioned this pull request May 12, 2018
@vks
Copy link
Collaborator Author

vks commented May 12, 2018

The documentation bits of this are superseded by #443. I think the only thing left is cleaning up the examples / CI scripts.

@dhardy
Copy link
Member

dhardy commented May 14, 2018

Essentially this is a compromise between code simplicity/clarity and making the tests work. These changes to the examples require a few tweaks to scripts e.g.:

cross test --tests --no-default-features --target mips-unknown-linux-gnu

to (I guess):

cross test --lib --no-default-features --target mips-unknown-linux-gnu

But I'm not sure it's worth picking a different compromise than what we already have?

@dhardy
Copy link
Member

dhardy commented May 16, 2018

So it sounds like there's no point making these changes.

@dhardy dhardy closed this May 16, 2018
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.

3 participants