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

Display doc comments for private types #1042

Merged

Conversation

mattgathu
Copy link
Contributor

Fixes #911

  • Adds a [package.metadata.docs.rs] to Cargo.toml
  • Sets the rustdoc-args key to [ "--no-defaults --passes collapse-docs --passes unindent-comments" ]

Fixes rust-lang#911

- Adds a [package.metadata.docs.rs] to Cargo.toml
- Sets the rustdoc-args key to [ "--no-defaults --passes collapse-docs --passes unindent-comments" ]
@vignesh-sankaran
Copy link
Contributor

vignesh-sankaran commented Sep 6, 2017

Could you figure out if there's a way to test this? The difficulty is that this needs to work with docs.rs, and so far, a way I think we could test this is to run docs.rs locally, but I'm not fully across how crates.io and docs.rs are linked. Alternatively, we could just accept this PR and see if it works, and submit more PRs until it does. It is a pretty small change after all :).

@mattgathu
Copy link
Contributor Author

mattgathu commented Sep 6, 2017

@vignesh-sankaran I could try running docs.rs locally. Not sure how I would test my changes though.

Also CI tests are failing because crates.io uses the [project] field instead of [package]
in the cargo.toml

Do I have to add a [package] field that duplicates whats under [project] or can I rename project 👉 package. My local testing doesn't seem to break anything.

@onur
Copy link
Member

onur commented Sep 6, 2017

@vignesh-sankaran unfortunately installing docs.rs won't allow you to test this PR, docs.rs currently only works with crates released into crates.io, there is no way to test any crate that is not released.

@vignesh-sankaran
Copy link
Contributor

@onur So a locally running docs.rs instance won't work with a local crates.io instance?

As a matter of interest, what's the mechanism docs.rs gets notified by of an updated crate version?

@vignesh-sankaran
Copy link
Contributor

@mattgathu Yes I remember running into this issue, I'm afraid I'm not sure how to work around this problem and I won't be able to look into it for at least the next few days :(

@onur
Copy link
Member

onur commented Sep 6, 2017

@vignesh-sankaran docs.rs is fetching crates.io-index repository every minute and checking if there is any difference. That is how docs.rs is getting aware of new crate releases.

Your second suggestion makes more sense. Docs.rs is actually a pretty basic program it's just checking new releases and running cargo doc on them. If given rustdoc args works on your machine, it will also work on docs.rs.

@vignesh-sankaran
Copy link
Contributor

@onur Would we need to point docs.rs at the local crates.io index? If so, how would we do this? Admittedly there is the immediate problem of trying to get the Cargo.toml to be valid, I'm not sure how to get it to work. Any ideas?

@mattgathu We're trying to figure out how you can go ahead and test this :).

@vignesh-sankaran
Copy link
Contributor

@mattgathu Did you try compiling it on your machine? You can check if the Cargo.toml is valid by running cargo check :)

@mattgathu
Copy link
Contributor Author

@vignesh-sankaran @onur great. thanks a lot for the help.

In the meantime I have renamed package.metadata.docs.rs to project.metadata.docs.rs. This clears the error when running cargo test.

@mattgathu
Copy link
Contributor Author

Any ideas on how to test this?

@kureuil
Copy link
Contributor

kureuil commented Sep 6, 2017

I'd say that the quick'n'dirty solution would be publishing this branch on crates.io under another name and see how it renders on docs.rs ?

Then we can yank it and not hear from it again.

@carols10cents
Copy link
Member

Thank you for the PR @mattgathu !!!

I'm up for just publishing this, and if it doesn't work, we'll publish another version! version numbers are free!!

@carols10cents
Copy link
Member

Found out from this comment that the command to use rustdoc in this case with arguments like these is actually:

cargo rustdoc --lib -- --no-defaults --passes collapse-docs --passes unindent-comments

I ran this locally and it completed successfully and documented private types, so I think this is worth a try! I'm going to have bors merge this in and then I'll bump the version number and publish :)

bors: r+

bors-voyager bot added a commit that referenced this pull request Sep 7, 2017
1042: Display doc comments for private types r=carols10cents

Fixes #911

- Adds a [package.metadata.docs.rs] to Cargo.toml
- Sets the rustdoc-args key to [ "--no-defaults --passes collapse-docs --passes unindent-comments" ]
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Sep 7, 2017

Build failed

@carols10cents
Copy link
Member

bors: r+

bors-voyager bot added a commit that referenced this pull request Sep 7, 2017
1042: Display doc comments for private types r=carols10cents

Fixes #911

- Adds a [package.metadata.docs.rs] to Cargo.toml
- Sets the rustdoc-args key to [ "--no-defaults --passes collapse-docs --passes unindent-comments" ]
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Sep 7, 2017

Build succeeded

@bors-voyager bors-voyager bot merged commit 37a624b into rust-lang:master Sep 7, 2017
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.

5 participants