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

Module doc examples #76671

Closed

Conversation

GuillaumeGomez
Copy link
Member

I was going through the libcore and adding missing code examples when I realized the lint was mostly complaining about modules. In my opinion, forcing modules to have code examples doesn't make sense.

What do you think @rust-lang/rustdoc ?

@rust-highfive
Copy link
Collaborator

r? @jyn514

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2020
@jyn514 jyn514 added A-doctests Area: Documentation tests, run by rustdoc A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 13, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 13, 2020

Can you give an example of the modules that were missing documentation? In general I think it's good to have; for instance https://docs.rs/cranelift-frontend/0.66.0/cranelift_frontend/#example is extremely helpful in seeing how to use the API. Module-level examples are good for showing how to put different parts of the API together, not just what individual functions do (which sometimes isn't very helpful on its own, e.g. https://docs.rs/cranelift-frontend/0.66.0/cranelift_frontend/struct.FunctionBuilder.html#method.inst_results).

I did spot that https://doc.rust-lang.org/stable/std/default/index.html is missing an example, but that's because all the docs are on the trait itself, not the module. So maybe it makes sense to move that to the module instead?

@Manishearth
Copy link
Member

Modules often have code examples in my experience. I think we should lint about them. It's okay to duplicate code examples between a module and its contents

@GuillaumeGomez
Copy link
Member Author

But in the case of a crate providing utilities and tools, it seems a bit weird. For example, libstd and libcore provide only tools. What should we put as example at the crate top level doc comment for example?

@jyn514
Copy link
Member

jyn514 commented Sep 13, 2020

If it doesn't make sense, then it should use allow(missing_doc_code_examples). But I don't think that means rustdoc shouldn't ever lint for modules.

@GuillaumeGomez
Copy link
Member Author

That would make a lot of allow(missing_doc_code_examples) in both libstd and libcore. ;)

@Manishearth
Copy link
Member

@GuillaumeGomez What we might want is an attribute that allows you to skip linting for just a module but not its contents.

@GuillaumeGomez
Copy link
Member Author

That'd still make a lot of module in both libcore and libstd. I really think that in some cases, modules don't need code examples.

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

I don't think rustdoc is the right place to fix this. If libstd doesn't need module examples, then it shouldn't enable the lint - it's off by default anyway. There are a lot of modules that do need linting and those shouldn't be ignored because it's not a good fit for libstd.

@GuillaumeGomez
Copy link
Member Author

I just used libstd as an example. But I'm pretty sure that it's more common to have a module that shouldn't be commented than the opposite.

@Manishearth
Copy link
Member

But I'm pretty sure that it's more common to have a module that shouldn't be commented than the opposite.

I disagree: Most modules I've worked on have a reasonable code example that could be used in their documentation. At worst, it's just a copy of a code example from a substruct.

I'm okay with adding a doc attribute that lets you skip this lint for a module, given that there is no way to apply allow() to a module and not have it apply to the children.

@GuillaumeGomez
Copy link
Member Author

Modules can be used as code namespaces (which is, I think, the most common thing). Adding a code example on something like that doesn't make sense, since it's just providing a set of functions/types. Which one should you pick? It doesn't make sense.

@Manishearth
Copy link
Member

When modules are used as code namespaces they are private with reexported members. If the module is public, it's being used as a proper namespace.

@Manishearth
Copy link
Member

Like, I'm struggling to come up with a project I have worked on where there is no applicable code example on the module, aside from grab-bag utility modules, or modules containing a lot of subparts (e.g. std).

@GuillaumeGomez
Copy link
Member Author

Yes, I realized it afterwards (forgot to write it here, just talked about that with @jyn514 on discord...).

@jyn514
Copy link
Member

jyn514 commented Sep 16, 2020

So the discussion is in one place: what we discussed is that we were talking at cross-purposes. @GuillaumeGomez was only referring to private modules and both @Manishearth and I thought he was talking about public modules. @GuillaumeGomez plans to make a few other PRs that make the handling of private vs public lints in rustdoc more consistent (of which I think #76767 is the first).

@bors
Copy link
Contributor

bors commented Sep 16, 2020

☔ The latest upstream changes (presumably #76804) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2020
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2020
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 8, 2020

@GuillaumeGomez does this still make sense to add? I think the discussion about missing_docs being consistent with or without --document-private-items is also true here: I would expect a separate lint for warning about private items, instead of it changing behavior based on a CLI argument.

@GuillaumeGomez
Copy link
Member Author

No, indeed! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants