Skip to content

rustdoc: tweak notes on ignore/compile_fail examples#45815

Merged
bors merged 2 commits intorust-lang:masterfrom
QuietMisdreavus:happy-little-notices
Nov 14, 2017
Merged

rustdoc: tweak notes on ignore/compile_fail examples#45815
bors merged 2 commits intorust-lang:masterfrom
QuietMisdreavus:happy-little-notices

Conversation

@QuietMisdreavus
Copy link
Contributor

Part of #44927

This is a softening of these notices to mention why a given example has a given callout, rather then telling viewers to be careful with an example. It also changes the character used for these samples from a warning logo to a circle-i/information logo.

image

@rust-highfive
Copy link
Contributor

r? @frewsxcv

(rust_highfive has picked a reviewer for you, use r? to override)

@QuietMisdreavus
Copy link
Contributor Author

QuietMisdreavus commented Nov 6, 2017

@rust-lang/docs These strings are being stirred up again, what do you think of these updates?

});
let tooltip = if ignore {
Some(("Be careful when using this code, it's not being tested!", "ignore"))
Some(("This example is not tested", "ignore"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of this new sentence. :-/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's matter of fact, much better.

Some(("This example is not tested", "ignore"))
} else if compile_fail {
Some(("This code doesn't compile so be extra careful!", "compile_fail"))
Some(("This example deliberately fails to compile", "compile_fail"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@QuietMisdreavus
Copy link
Contributor Author

Oh right, there's a test for the warning text, so travis failed because i didn't change it:

[00:53:03] 15: @has check failed
[00:53:03] 	`XPATH PATTERN` did not match
[00:53:03] 	// @has foo/fn.bar.html '//*[@class="tooltip compile_fail"]/span' "This code doesn't compile so be extra careful!"
[00:53:03] 16: @has check failed
[00:53:03] 	`XPATH PATTERN` did not match
[00:53:03] 	// @has foo/fn.bar.html '//*[@class="tooltip ignore"]/span' "Be careful when using this code, it's not being tested!"
[00:53:03] 
[00:53:03] Encountered 2 errors
[00:53:03] 
[00:53:03] ------------------------------------------
[00:53:03] 
[00:53:03] thread '[rustdoc] rustdoc/codeblock-title.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2498:8

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 7, 2017
Copy link
Contributor

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! the text associated with the examples is a bit subjective, but i personally think this is an improvement. r=me with test fixes and if imperio is cool with the changes

@carols10cents
Copy link
Member

triage ping for you @QuietMisdreavus! ❤️

@QuietMisdreavus
Copy link
Contributor Author

I've pushed a commit that fixes the test.

@GuillaumeGomez Do you have any specific concerns about the new text, or are you okay with this landing? AFAIK @frewsxcv wanted to make sure you were okay with the changes before approving.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2017
@GuillaumeGomez
Copy link
Member

Oops sorry! The current text is fine for me so let's merge.

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 13, 2017

📌 Commit 02b3785 has been approved by GuillaumeGomez

@kennytm kennytm 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 Nov 14, 2017
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 14, 2017
…s, r=GuillaumeGomez

rustdoc: tweak notes on ignore/compile_fail examples

Part of rust-lang#44927

This is a softening of these notices to mention *why* a given example has a given callout, rather then telling viewers to be careful with an example. It also changes the character used for these samples from a warning logo to a circle-i/information logo.

![image](https://user-images.githubusercontent.com/5217170/32464361-5fbb5d9e-c305-11e7-8482-ce71b97a54df.png)
bors added a commit that referenced this pull request Nov 14, 2017
Rollup of 7 pull requests

- Successful merges: #45815, #45941, #45950, #45951, #45961, #45967, #45970
- Failed merges:
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 14, 2017
…s, r=GuillaumeGomez

rustdoc: tweak notes on ignore/compile_fail examples

Part of rust-lang#44927

This is a softening of these notices to mention *why* a given example has a given callout, rather then telling viewers to be careful with an example. It also changes the character used for these samples from a warning logo to a circle-i/information logo.

![image](https://user-images.githubusercontent.com/5217170/32464361-5fbb5d9e-c305-11e7-8482-ce71b97a54df.png)
bors added a commit that referenced this pull request Nov 14, 2017
Rollup of 7 pull requests

- Successful merges: #45815, #45941, #45950, #45961, #45967, #45970, #45977
- Failed merges:
@bors bors merged commit 02b3785 into rust-lang:master Nov 14, 2017
@QuietMisdreavus QuietMisdreavus deleted the happy-little-notices branch February 26, 2018 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants