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

Add Crate level documentation and Cargo attributes #438

Merged
merged 4 commits into from Sep 26, 2017

Conversation

Projects
None yet
3 participants
@AndyGauge
Copy link
Contributor

AndyGauge commented Sep 12, 2017

Fixes #418
Fixes #423

@nikomatsakis

This comment has been minimized.

Copy link
Member

nikomatsakis commented Sep 18, 2017

@AndyGauge I see some travis failures; I suspect they are unrelated to this PR, but would you mind rebasing?

@AndyGauge AndyGauge force-pushed the AndyGauge:docs branch from b225650 to 1843312 Sep 22, 2017

@AndyGauge

This comment has been minimized.

Copy link
Contributor Author

AndyGauge commented Sep 22, 2017

There's doc code that needs to be modified. I'll figure it out.

@AndyGauge AndyGauge force-pushed the AndyGauge:docs branch from 1843312 to 64e464d Sep 22, 2017

@AndyGauge AndyGauge force-pushed the AndyGauge:docs branch from 64e464d to 0fcce4a Sep 22, 2017

@AndyGauge

This comment has been minimized.

Copy link
Contributor Author

AndyGauge commented Sep 23, 2017

OK, this is ready for review.

Thanks

@cuviper
Copy link
Member

cuviper left a comment

Thanks! I just have a few comments.

src/lib.rs Outdated
//!
//! # Parallel Iterators
//!
//! Parallel iterators are formed using [`par_iter`] and `par_iter_mut` functions

This comment has been minimized.

@cuviper

cuviper Sep 23, 2017

Member

It's strange to me that you've linked par_iter to the ParallelIterator trait. It is good to have that link up front, but par_iter itself comes from IntoParallelRefIterator. There are actually three such conversions, into_par_iter(), par_iter(), and par_iter_mut(), to iterate respectively by value, by shared reference, or by mutable reference.

src/lib.rs Outdated
//! _ => *c
//! }
//! }).collect();
//! # }

This comment has been minimized.

@cuviper

cuviper Sep 23, 2017

Member

It would be good to assert_eq! the result so readers know what to expect, and so it also serves as a doctest.

src/lib.rs Outdated
//! ```
//!
//! Here a string is encrypted using ROT13 leveraging parallelism. Once all the
//! threads are complete, they are collected into a string.

This comment has been minimized.

@cuviper

cuviper Sep 23, 2017

Member

I think it reads better to place such descriptions before the example code.

@@ -8,6 +8,9 @@ description = "Simple work-stealing parallelism for Rust"
license = "Apache-2.0/MIT"
repository = "https://github.com/nikomatsakis/rayon"
documentation = "https://docs.rs/rayon/"
readme = "README.md"
keywords = ["parallel", "thread", "concurrency", "join", "performance"]
categories = ["concurrency"]

This comment has been minimized.

@cuviper

cuviper Sep 23, 2017

Member

Can you also add these attributes to rayon-core and rayon-futures?

This comment has been minimized.

@AndyGauge

AndyGauge Sep 25, 2017

Author Contributor

I'm not sure if readme attribute is appropriate. I added '..\readme.md'. Not sure if that is helpful or if the other 'crates' need their own readmes.

This comment has been minimized.

@cuviper

cuviper Sep 26, 2017

Member

I have no idea if crates.io will work with ../README.md -- I doubt it though, because it's probably looking at the individual uploaded crates, not the repository.

We do have a readme in rayon-futures, but not rayon-core. The latter could just be a simple placeholder, saying something like "Core APIs for Rayon; see the rayon crate for more details."

@cuviper

This comment has been minimized.

Copy link
Member

cuviper commented Sep 26, 2017

Thanks!

@cuviper cuviper merged commit 0503550 into rayon-rs:master Sep 26, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.