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

rustdoc: Note why rustdoc::html::markdown is public #81492

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Jan 28, 2021

Almost all of the modules are crate-private, except for
rustdoc::json::types, which I believe is intended to be for public
use; and rustdoc::html::markdown, which is used externally by the
error-index generator and so has to be public.

r? @GuillaumeGomez

@camelid camelid added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 28, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2021
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
 finished in 0.426 seconds
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 29 tests
iiiiiiiiiiiiiiiiiiiiiiiiiiiii

Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
 finished in 0.072 seconds
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii......ii...ii...........iiii........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.29s

 finished in 2.370 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
running 1 test
F
failures:

---- html/markdown.rs - html::markdown (line 5) stdout ----
error[E0603]: module `html` is private
  --> html/markdown.rs:11:14
   |
9  | use rustdoc::html::markdown::{IdMap, Markdown, ErrorCodes};
   |              ^^^^ private module
   |
note: the module `html` is defined here
   |
   |
87 | mod html;

error: aborting due to previous error

For more information about this error, try `rustc --explain E0603`.
---

error: test failed, to rerun pass '--doc'


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/rustdoc/Cargo.toml" "-p" "rustdoc:0.0.0" "--" "--quiet"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:27:37

@camelid
Copy link
Member Author

camelid commented Jan 29, 2021

This doctest is failing as a result of this change:

---- html/markdown.rs - html::markdown (line 5) stdout ----
error[E0603]: module `html` is private
  --> html/markdown.rs:11:14
   |
9  | use rustdoc::html::markdown::{IdMap, Markdown, ErrorCodes};
   |              ^^^^ private module

Should I just mark it as ignore?

@bugadani
Copy link
Contributor

Should I just mark it as ignore?

It certainly doesn't seem to test anything, so I think you should.

@GuillaumeGomez
Copy link
Member

Yes, you can ignore it.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@camelid
Copy link
Member Author

camelid commented Feb 1, 2021

It turns out rustdoc::html is used by error_index_generator, so I don't think we can make it private. (That may also be why there was a doctest — to test that the basic interface stayed somewhat stable, or to show that it's used externally.) I think I'll make it public again and un-ignore the doctest.

@camelid camelid force-pushed the rustdoc-internal-mod-vis branch 3 times, most recently from 367a31f to a5841e6 Compare February 1, 2021 03:21
Almost all of the modules are crate-private, except for
`rustdoc::json::types`, which I believe is intended to be for public
use; and `rustdoc::html::markdown`, which is used externally by the
error-index generator and so has to be public.
@camelid camelid changed the title rustdoc: Make some modules crate-private rustdoc: Note why rustdoc::html::markdown is public Feb 1, 2021
@camelid
Copy link
Member Author

camelid commented Feb 1, 2021

At this point, I'm no longer changing the visibility of any the modules, so I updated the commit message and PR title and description :)

@GuillaumeGomez
Copy link
Member

Well, more doc is always a good thing, so thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Feb 1, 2021

📌 Commit 82010e8 has been approved by GuillaumeGomez

@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 Feb 1, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Feb 2, 2021
…=GuillaumeGomez

rustdoc: Note why `rustdoc::html::markdown` is public

Almost all of the modules are crate-private, except for
`rustdoc::json::types`, which I believe is intended to be for public
use; and `rustdoc::html::markdown`, which is used externally by the
error-index generator and so has to be public.

r? `@GuillaumeGomez`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2021
…as-schievink

Rollup of 11 pull requests

Successful merges:

 - rust-lang#80629 (Add lint for 2229 migrations)
 - rust-lang#81022 (Add Frames Iterator for Backtrace)
 - rust-lang#81481 (move some tests)
 - rust-lang#81485 (Add some tests for associated-type-bounds issues)
 - rust-lang#81492 (rustdoc: Note why `rustdoc::html::markdown` is public)
 - rust-lang#81577 (const_evaluatable: consider sub-expressions to be evaluatable)
 - rust-lang#81599 (Implement `TrustedLen` for `Fuse<I: TrustedLen>`)
 - rust-lang#81608 (Improve handling of spans around macro result parse errors)
 - rust-lang#81609 (Remove the remains of query categories)
 - rust-lang#81630 (Fix overflowing text on mobile when sidebar is displayed)
 - rust-lang#81631 (Remove unneeded `mut` variable)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 30f12a0 into rust-lang:master Feb 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 2, 2021
@camelid camelid deleted the rustdoc-internal-mod-vis branch February 2, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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
Development

Successfully merging this pull request may close these issues.

7 participants