make sentence more simple #2561
Conversation
|
Thanks for the PR. If you have write access, feel free to merge this PR if it does not need reviews. You can request a review using |
rustbot
added
the
S-waiting-on-review
src/tests/compiletest.md
Outdated
| @@ -90,7 +90,8 @@ The following test suites are available, with links for more information: | |||
| | `rustdoc-ui` | Check terminal output of `rustdoc` ([see also](ui.md)) | | |||
|
|
|||
| Some rustdoc-specific tests can also be found in `ui/rustdoc/`. | |||
| These check rustdoc-related or -specific lints that (also) run as part of `rustc`, not (only) `rustdoc`. | |||
| These check lints that are either related or specific to rustdoc, | |||
| that also run as part of `rustc`, not (only) `rustdoc`. | |||
There was a problem hiding this comment.
this was not a great sentence even before your change.
some background:
- bootstrap allows abbreviating Step paths named x/y/z as z. if there are multiple paths that end with z, it effectively picks a random path (it’s deterministic, but not predictable without reading the code).
this sentence is using rustdoc in a way that makes it look like it refers to x test tests/ui/rustdoc. i am not convinced that is even the suite path that runs when you say x test rustdoc, and i have no idea what x test rustc does, maybe unit tests for rustc-main (i.e. effectively a no-op).
i think the intent is actually to refer to the CLI tools themselves, not the paths passed to bootstrap, but that’s very unclear from context.
tests/ui/rustdocis running rustc, not rustdoc.tests/ui/rustdoc/README.mdsays this clearly IMO:
This directory is for tests that have to do with rustdoc, but test the behavior
of rustc. For example, rustc should not warn that an attribute rustdoc uses is
unknown.
i would suggest something like this:
| that also run as part of `rustc`, not (only) `rustdoc`. | |
| These check lints and behaviors *of rustc* that are related to rustdoc. Note that [not all lints emitted by rustc are emitted by rustdoc](https://rustc-dev-guide.rust-lang.org/rustdoc.html#constraints). |
There was a problem hiding this comment.
I guess I wrote these sentences and they're indeed quite poorly worded, oh my! I must've written them with a tired brain powering through the endless iterations of PR #2298 I simply wanted to have merged and be done with :'D
There was a problem hiding this comment.
This isn't meant to refer to specific compiletest test suites, with "lints that are run as part of $program" I was trying to express the fact that some lints are [emitted] / some lint [impls and/or passes] are run by both the binaries, rustc and rustdoc and we want to ensure that they (i.e., a specific subset of lints) are not only emitted / run when executing rustdoc but also when executing rustc.
tshepang
force-pushed
the
tshepang-patch-1
branch
from
646de95 to
bf76b8c
Compare
|
@fmease are you happy with latest change |
| @@ -90,7 +90,8 @@ The following test suites are available, with links for more information: | |||
| | `rustdoc-ui` | Check terminal output of `rustdoc` ([see also](ui.md)) | | |||
|
|
|||
| Some rustdoc-specific tests can also be found in `ui/rustdoc/`. | |||
| These check rustdoc-related or -specific lints that (also) run as part of `rustc`, not (only) `rustdoc`. | |||
| These tests ensure that lints that are emitted as part of executing rustdoc | |||
There was a problem hiding this comment.
Not sure how to rephrase this, but it's not (all) lints, it's only some lints. There are lints that are only emitted by rustdoc, not rustc, like the ones prefixed with rustdoc::.
That's already miles better, thank you! It's still not quite correct (see inline review comment). |
|
am happy if you can push directly to the pr... I imagine you can |
I don't understand what "not (only)
rustdoc" means