Remove most "```ignore" doc tests. #42777
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
| Additional info:
|
| Amazing! I have always wanted to do this. Thank you so much! I can't review right this moment, but was too excited not to leave a message 💯 |
| Looks like Travis failed. |
kennytm force-pushed the kill-ignore-doctest branch from 0712a27 to 524a892 Compare | @Mark-Simulacrum Thanks. Turned this back into |
arielb1 added the S-waiting-on-review 
estebank approved these changes Jun 21, 2017
src/libcore/ptr.rs Outdated
There was a problem hiding this comment.
What's the reasoning behind this change?
There was a problem hiding this comment.
@estebank Reverted. Thought this was ```ignored because &10u8 would be dropped too early (it was ```ignored to avoid introducing a #[feature] to the example when .as_ref() was unstable).
src/libcore/ptr.rs Outdated
src/librustc/diagnostics.rs Outdated
There was a problem hiding this comment.
Should the unsafe block visible to the documentation?
There was a problem hiding this comment.
May be these two examples should be separated into a code block that succeeds (above) and one that fails with E0381 (bellow).
There was a problem hiding this comment.
@estebank Separated into 3 code blocks.
src/librustc_borrowck/diagnostics.rs Outdated
There was a problem hiding this comment.
Separate the success and failure cases to catch any regressions. For the second block it'll need a bunch of duplicated hidden code :-/
There was a problem hiding this comment.
@estebank Reworded and separated into 2 examples.
src/librustc_typeck/diagnostics.rs Outdated
There was a problem hiding this comment.
Why is this description commented out? Add a reason to the comment to avoid confusion.
There was a problem hiding this comment.
@estebank Added a comment to explain why this is hidden at the moment.
kennytm force-pushed the kill-ignore-doctest branch from 524a892 to 7edba12 Compare | @estebank Thanks! All addressed. |
| @bors r+ rollup |
| 📌 Commit 7edba12 has been approved by |
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 22, 2017
…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 22, 2017
…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.
| @estebank |
| @bors r- |
| @bors r+ rollup |
| 📌 Commit 5dd6538 has been approved by |
| @kennytm can you file a ticket for that? We need to figure out what the actual problem is, specially because we don't want to have documentation that is incorrect or misleading. It should contain code to reproduce the problem under Windows, and a link to the workaround commit in this PR. Probably the output of AppVeyor too: |
kennytm force-pushed the kill-ignore-doctest branch 2 times, most recently from 5612473 to 513b962 Compare | @bors r- |
| @bors r+ |
| 📌 Commit 513b962 has been approved by |
| Canceled try build. |
| Wowee. Super cleanup patch @kennytm. |
Mark-Simulacrum added S-waiting-on-bors | @bors r+ retry |
| 💡 This pull request was already approved, no need to approve it again.
|
| 📌 Commit 513b962 has been approved by |
bors added a commit that referenced this pull request Jun 22, 2017
Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. #42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.
| ah, whatever, I guess try actually has to pass for bors to be happy.. @bors r=estebank |
| 💡 This pull request was already approved, no need to approve it again.
|
| 📌 Commit 513b962 has been approved by |
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 22, 2017
…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.
| @bors r- |
| ☀️ Test successful - status-travis |
Replaced by adding extra imports, adding hidden code (`# ...`), modifying examples to be runnable (sorry Homura), specifying non-Rust code, and converting to should_panic, no_run, or compile_fail. Remaining "```ignore"s received an explanation why they are being ignored.
kennytm force-pushed the kill-ignore-doctest branch from 513b962 to 9addd3b Compare | @Mark-Simulacrum Rebased to remove the two |
| @bors r=estebank |
| 📌 Commit 9addd3b has been approved by |
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 23, 2017
…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.
bors added a commit that referenced this pull request Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Unconditional
```ignoredoc tests lead to outdated examples (e.g. #42729 (comment)). This PR tries to change all existing```ignoretests into one of the following:```text/```sh/```json/```dot```compile_fail```should_panic```no_run```ignoreThe
--explainhandling is changed to cope with hidden lines from the error index.Tidy is changed to reject any unexplained
```ignoreand```rust,ignore.