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

Build crates with minimal versions in CI #1190

Merged
merged 2 commits into from
Aug 10, 2018

Conversation

MajorBreakfast
Copy link
Contributor

Follow-up PR to #1189

The CI should break if we're relying on features not available in the minimal versions we specify.

cc @cramertj

@MajorBreakfast MajorBreakfast requested a review from Nemo157 August 10, 2018 06:44
- cargo build --manifest-path futures-executor/Cargo.toml --all-features
- cargo build --manifest-path futures-io/Cargo.toml --all-features
- cargo build --manifest-path futures-sink/Cargo.toml --all-features
- cargo build --manifest-path futures-util/Cargo.toml --all-features
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we use cargo build --all-features or cargo build -p futures-... --all-features instead?

cargo build --no-default-features seems not to exist at least

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cargo build --all-features might also not work. It exists but I'm not sure whether it does what we want

Copy link
Member

Choose a reason for hiding this comment

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

Nope, features and workspaces are pretty much broken at the moment. There was an attempt at fixing them a while ago, but I’m not sure if that’s still available even as an unstable flag. I’ll try and find the link to the issue in a sec.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested -p instead of --manifest-path and it doesn't work

@MajorBreakfast MajorBreakfast merged commit 669cf78 into rust-lang:master Aug 10, 2018
@cramertj
Copy link
Member

Nice!

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