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

use std by default #130

Merged
merged 10 commits into from
Feb 4, 2018
Merged

use std by default #130

merged 10 commits into from
Feb 4, 2018

Conversation

kinggoesgaming
Copy link
Member

closes #123

Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
@kinggoesgaming
Copy link
Member Author

kinggoesgaming commented Feb 3, 2018

documentation still needs to be updated

Edit: done

@kinggoesgaming kinggoesgaming self-assigned this Feb 3, 2018
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
@kinggoesgaming kinggoesgaming changed the title wip use std by default use std by default Feb 3, 2018
@kinggoesgaming kinggoesgaming changed the title use std by default wip use std by default Feb 4, 2018
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
added NDF variable, for --no-default-features

Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
@kinggoesgaming kinggoesgaming changed the title wip use std by default use std by default Feb 4, 2018
@kinggoesgaming kinggoesgaming added this to the 1.0.0 milestone Feb 4, 2018
src/lib.rs Outdated
//! This crate by default has no dependencies and is `#![no_std]` compatible.
//! The following Cargo features, however, can be used to enable various pieces
//! of functionality.
//! By default, this crate depends on nothing but `libstd` and cannot generate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We could probably just say std instead of libstd.

src/lib.rs Outdated
//! of functionality.
//! By default, this crate depends on nothing but `libstd` and cannot generate
//! any `Uuid`s. You need to enable the following Cargo features to enable
//! various pieces of functionality:
//!
//! * `std` - adds in functionality available when linking to the standard
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably just remove std from this list, since we're implying that you need to enable these features to get functionality,

src/lib.rs Outdated
// support in those situations as well.
#[cfg(any(feature = "std",
feature = "serde"))]
extern crate core;
Copy link
Member

@KodrAus KodrAus Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work when compiling with no_std. Instead we might need to do something like:

#[cfg(feature = "std")]
extern crate std as core;

EDIT: Nevermind, you fixed it already 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, let's try and sort out our std features. Right now, if you specify --no-default-features --features serde you'll get a compile error from duplicate uses of core.

We never need anything from serde that uses std though, so we could specify it in our Cargo.toml as:

serde = { version = "1.0.16", optional = true, default-features = false }

And then include core in lib.rs as:

#[cfg(feature = "std")]
extern crate std as core;

Then no_std users can use uuid with serde.

Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
@kinggoesgaming
Copy link
Member Author

@KodrAus this should ready to go unless you any more nitpicks :)

@KodrAus
Copy link
Member

KodrAus commented Feb 4, 2018

Thanks @kinggoesgaming! Just one more comment:

We can change the attribute pulling in the std_support module to only require the std feature instead of also including serde.

Signed-off-by: Hunar Roop Kahlon <hunar.roop@gmail.com>
@kinggoesgaming
Copy link
Member Author

@KodrAus done

@KodrAus
Copy link
Member

KodrAus commented Feb 4, 2018

This looks good to me!

bors r+

bors bot added a commit that referenced this pull request Feb 4, 2018
130: use std by default r=KodrAus a=kinggoesgaming

closes #123
@bors
Copy link
Contributor

bors bot commented Feb 4, 2018

@bors bors bot merged commit b3684ee into master Feb 4, 2018
@kinggoesgaming kinggoesgaming deleted the default-std branch February 4, 2018 19:23
@kinggoesgaming kinggoesgaming modified the milestones: 1.0.0, 0.6.0 Feb 5, 2018
bors bot added a commit that referenced this pull request Feb 5, 2018
131: remove extern crate std in serde r=KodrAus a=kinggoesgaming

This somehow was missed in #130
@kinggoesgaming kinggoesgaming modified the milestones: 0.6.0, 0.6.0-beta Feb 11, 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.

Use std by default
2 participants