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

Add suggestion to missing backticks error #7904

Merged
merged 2 commits into from
Nov 2, 2021
Merged

Add suggestion to missing backticks error #7904

merged 2 commits into from
Nov 2, 2021

Conversation

Serial-ATA
Copy link
Contributor

@Serial-ATA Serial-ATA commented Oct 31, 2021 •
edited
Loading

changelog: Add a machine applicable suggestion for the [doc_markdown] missing backticks lint

closes: #7737

@rust-highfive
Copy link

r? @xFrednet

(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 Oct 31, 2021
xFrednet
xFrednet reviewed Oct 31, 2021
View reviewed changes
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, the changes look good in general, just some minor NITs. 🙃

tests/ui/doc/doc.stderr Outdated Show resolved Hide resolved
tests/ui/doc/doc.stderr Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
@Serial-ATA Serial-ATA force-pushed the improve-doc-suggestion branch from fcef581 to 34b2477 Compare October 31, 2021 16:04
@Serial-ATA Serial-ATA force-pushed the improve-doc-suggestion branch 2 times, most recently from 01ac004 to f37dc25 Compare November 2, 2021 02:07
xFrednet
xFrednet reviewed Nov 2, 2021
View reviewed changes
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

Alright, I found two more smaller things where it would be great if you could address them. Then I would merge it. Thank you for the quick changes 🙃

tests/ui/doc/doc.stderr Outdated Show resolved Hide resolved
tests/ui/doc/missing_ticks.rs Outdated Show resolved Hide resolved
tests/ui/doc/issue_1832.rs
@@ -0,0 +1,9 @@
/// Ok: <http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels>
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why you extracted these two tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those two tests fail with rustfix because they have a suggestion that isn't machine applicable, and it's compiled with -Dwarnings.

rust-clippy/tests/compile-test.rs

Lines 131 to 132 in c68af43

config.target_rustcflags = Some(format!(
"--emit=metadata -Dwarnings -Zui-testing -L dependency={}{}{}",

Copy link
Member

Choose a reason for hiding this comment

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

That explains why some lint test files are split up into a fixable and not fixable test file. Today I learned. Would you mind renaming the doc.rs file to doc-fixable.rs? You can also do that in an additional commit ^^ then everything is really ready 🙃

@xFrednet xFrednet mentioned this pull request Nov 2, 2021
Closed
@bors
Copy link
Contributor

bors commented Nov 2, 2021

☔ The latest upstream changes (presumably #7916) made this pull request unmergeable. Please resolve the merge conflicts.

@Serial-ATA Serial-ATA force-pushed the improve-doc-suggestion branch from f37dc25 to 0db8c52 Compare November 2, 2021 20:51
@xFrednet
Copy link
Member

xFrednet commented Nov 2, 2021 •
edited
Loading

Fantastic! Thank you very much for addressing my sometimes nit picky comments. I appreciate it! 🙃

This looks excellent to me now 👍

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2021

📌 Commit 3732d11 has been approved by xFrednet

@bors
Copy link
Contributor

bors commented Nov 2, 2021

⌛ Testing commit 3732d11 with merge 4d26c5c...

@bors
Copy link
Contributor

bors commented Nov 2, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 4d26c5c to master...

@bors bors merged commit 4d26c5c into rust-lang:master Nov 2, 2021
@Serial-ATA Serial-ATA deleted the improve-doc-suggestion branch November 2, 2021 21:20
@xFrednet xFrednet mentioned this pull request Nov 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giraffate giraffate giraffate left review comments

@xFrednet xFrednet xFrednet approved these changes

Assignees

@xFrednet xFrednet

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add functionality to automatically put variable names in markdown docs in ticks
5 participants
@Serial-ATA @rust-highfive @bors @xFrednet @giraffate