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

adopt stricter semver policy around unstable features #368

Merged
merged 2 commits into from Jun 14, 2017
Merged

Conversation

nikomatsakis
Copy link
Member

@nikomatsakis nikomatsakis commented Jun 14, 2017

This branch documents our semver policy and also makes it less convenient to use unstable features. They are no longer exposed as a cargo feature, but instead require RUSTFLAGS=--cfg unstable.

Fixes #369.

@nikomatsakis nikomatsakis changed the title document how to use rayon as well as semver policy adopt stricter semver policy around unstable features Jun 14, 2017
Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

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

The policy seems good, just a few minor fixups.

One more thing is to update the allow(warnings) in both rayon and rayon-core, but I'd actually prefer to just drop that and start being more diligent in keeping clean.

README.md Outdated

To use the Parallel Iterator APIs, a number of traits have to be in
scope. The easiest way to bring those things into scope is to use the
[Rayon prelude](https://docs.rs/rayon/0.8.0/rayon/prelude/index.html).
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 lazily use * instead of the explicit version number here.

README.md Outdated

```rust
[dependencies]
rayon = XXX # <-- insert latest version of Rayon from crates.io here
Copy link
Member

Choose a reason for hiding this comment

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

Here I think we should fill in the current "0.8" semver, so users don't have to figure that out. It's a relatively small thing for us to update when we do bump semver.

README.md Outdated
that depends on your crate. This infectious nature is intentional, as
it serves as a reminder that you are outside of the normal semver
guarantees. **If you see unstable APIs that you would like to use,
please open an issue about stabilizing them!**
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just make it standard policy to open tracking issues for all unstable features.

Copy link
Member

Choose a reason for hiding this comment

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

It occurs to me that the infectious nature is actually a double-edged sword. Suppose one of my dependencies is happily using stable rayon-0.8 behind the scenes, but then I want to enable unstable rayon-0.9 / rayon-core-1.2 for my own purposes. Broadly enabling --cfg rayon_unstable will break that stable 0.8-using crate too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, interesting point. Still seems like an improvement relative to where we are now.

README.md Outdated
of the rayon crate can peacefully coexist; they will all share one
global thread-pool through the `rayon-core` crate.

## Unstable features## License
Copy link
Member

Choose a reason for hiding this comment

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

Misplaced header?

@nikomatsakis
Copy link
Member Author

OK, addressed your points.

Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

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

Looks good!

@cuviper cuviper merged commit 3033081 into master Jun 14, 2017
@cuviper
Copy link
Member

cuviper commented Jun 14, 2017

(the CI queue is totally stalled, so I'm assuming you did sanity-check the build...)

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.

None yet

2 participants