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.

Sign up for GitHub

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

Jump to bottom

Make 'docs' nullable in rustdoc-json output #81225

Merged
merged 3 commits into from
Jan 22, 2021
Merged

Make 'docs' nullable in rustdoc-json output #81225

merged 3 commits into from
Jan 22, 2021

Conversation

CraftSpider
Copy link
Contributor

Matches the backing better, and makes it so there's a difference between 'empty docs' and 'no docs'.

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(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 Jan 20, 2021
@GuillaumeGomez
Copy link
Member

Thanks! r=me once CI pass.

@aDotInTheVoid
Copy link
Member

aDotInTheVoid commented Jan 20, 2021 •
edited
Loading

Should the RFC be edited, or how should this be documented?

@CraftSpider
Copy link
Contributor Author

According to the tracking issue, another RFC may be written when things go stable. I was thinking until then it's not worth going through the RFC process for every unstable change

@CraftSpider
Copy link
Contributor Author

For documentation, I'm not sure. It seems there should be a better place than an RFC, but I'm not sure what that place is

@CraftSpider
Copy link
Contributor Author

According to the original RFC:
"Given that the JSON output will be implemented as a set of Rust types with serde serialization, the most useful docs for them would be the 40 or so types themselves. By writing docs on those types the Rustdoc page for that module would become a good reference. It may be helpful to provide some sort of schema for use with other languages".
So its advice is that the types and their doc comments are the whole of the documentation. I'd say a proper schema would be worthwhile, but not till the protocol is stable. So currently, this change is as documented as the RFC advises.
(Apologies for the multiple comments in a row, I added things as I thought of / found them)

@aDotInTheVoid
Copy link
Member

Using the rustdoc output seems good, 3 things to thing of.

  1. you should add a doc comment explaining Some("") vs None.
  2. The rfc should be edited to point to the internal docs or this crate which copyies the definions.
  3. Additionaly all the explanations from the RFC should be added as doc comments

I'll do 2 and 3 tomorrow

@jyn514
Copy link
Member

jyn514 commented Jan 21, 2021

The rfc should be edited to point to the internal docs or this crate which copyies the definions.

I do not think an external crate should be the canonical reference for the format. I see two ways forward here:

  • Autopublish src/librustdoc/json as its own crate, and have rustdoc depend on it. The crate will have normal semver, and will be pre-1.0 until the JSON format is stabilized. It may have major versions other than 1.0 as long as the format-version in the JSON output is changed.
  • Don't have a crate at all; have documentation instead and let people write external crates based on it. I don't like this quite as much, but it would work. I still don't think we should treat external crates as the canonical version, though.

Also, @CraftSpider can you bump format-version in the JSON output?

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 21, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 22, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jan 22, 2021

📌 Commit 26565f0 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2021
@m-ou-se m-ou-se mentioned this pull request Jan 22, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2021
@bors
Auto merge of rust-lang#81271 - m-ou-se:rollup-xv7gq3w, r=m-ou-se
b814b63 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#80573 (Deny rustc::internal lints for rustdoc and clippy)
 - rust-lang#81173 (Expand docs on Iterator::intersperse)
 - rust-lang#81194 (Stabilize std::panic::panic_any.)
 - rust-lang#81202 (Don't prefix 0x for each segments in `dbg!(Ipv6)`)
 - rust-lang#81225 (Make 'docs' nullable in rustdoc-json output)
 - rust-lang#81227 (Remove doctree::StructType)
 - rust-lang#81233 (Document why not use concat! in dbg! macro)
 - rust-lang#81236 (Gracefully handle loop labels missing leading `'` in different positions)
 - rust-lang#81241 (Turn alloc's force_expr macro into a regular macro_rules.)
 - rust-lang#81242 (Enforce statically that `MIN_NON_ZERO_CAP` is calculated at compile time)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1cc13b4 into rust-lang:master Jan 22, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 22, 2021
@aDotInTheVoid
Copy link
Member

@rustbot modify labels +A-rustdoc-json

@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jan 24, 2021
@CraftSpider CraftSpider deleted the json-opt-docs branch February 23, 2022 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.51.0
Development

Successfully merging this pull request may close these issues.
7 participants
@CraftSpider @rust-highfive @GuillaumeGomez @aDotInTheVoid @jyn514 @bors @rustbot