-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Place TaskPool comment so it documents struct #14993
Conversation
@@ -10,9 +10,6 @@ | |||
|
|||
#![allow(missing_doc)] | |||
|
|||
/// A task pool abstraction. Useful for achieving predictable CPU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is designed to be on the module, i.e. //! A task pool abstraction...
. (Having it both here and on the struct seems reasonable: the struct comment could theoretically be expanded to include examples and general discussion, but no need to do it here.)
Fixed, comment is now on module and struct. EDIT: Build successful, without |
It doesn't seem too useful to just copy the same documentation for both the module and the struct? It's not exactly the most useful documentation to start with and duplicating it seems like we're just blindly satisfying the goal of |
I agree. I initially just moved the comment because TaskPool is the only undocumented struct here: http://doc.rust-lang.org/std/sync/ I guess it's a better solution to actually write useful doc for the module and struct. I will see what I can do. |
I've went ahead and improved the whole module's documentation. My changes are explained in the lastest commit. |
Looks good to me, thanks! Could you squash the commits into one? |
The struct and module doc comments are reformulated. The `execute` method's documentation are put up to date, and failure information is added. A test is also added to address the possible failure.
Done :) |
TaskPool is the only item in the std::sync doc that doesn't have a summary. This fixes that problem.
TaskPool is the only item in the std::sync doc that doesn't have a summary. This fixes that problem.