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

Place TaskPool comment so it documents struct #14993

Merged
merged 1 commit into from
Jun 20, 2014
Merged

Place TaskPool comment so it documents struct #14993

merged 1 commit into from
Jun 20, 2014

Conversation

alxgnon
Copy link
Contributor

@alxgnon alxgnon commented Jun 18, 2014

TaskPool is the only item in the std::sync doc that doesn't have a summary. This fixes that problem.

@@ -10,9 +10,6 @@

#![allow(missing_doc)]

/// A task pool abstraction. Useful for achieving predictable CPU
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 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.)

@alxgnon
Copy link
Contributor Author

alxgnon commented Jun 18, 2014

Fixed, comment is now on module and struct.

EDIT: Build successful, without #![allow(missing_doc)]. Great!

@alexcrichton
Copy link
Member

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 #[deny(missing_doc)].

@alxgnon
Copy link
Contributor Author

alxgnon commented Jun 18, 2014

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.

@alxgnon
Copy link
Contributor Author

alxgnon commented Jun 19, 2014

I've went ahead and improved the whole module's documentation. My changes are explained in the lastest commit.

@alexcrichton
Copy link
Member

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.
@alxgnon
Copy link
Contributor Author

alxgnon commented Jun 20, 2014

Done :)

bors added a commit that referenced this pull request Jun 20, 2014
TaskPool is the only item in the std::sync doc that doesn't have a summary. This fixes that problem.
@bors bors closed this Jun 20, 2014
@bors bors merged commit af520e1 into rust-lang:master Jun 20, 2014
@alxgnon alxgnon deleted the taskpooldocfix branch June 20, 2014 07:50
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.

4 participants