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

Convert clippy_lints to intra-doc links #6412

Closed
wants to merge 2 commits into from
Closed

Convert clippy_lints to intra-doc links #6412

wants to merge 2 commits into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 1, 2020

This commit can (mostly) be replicated with cargo intraconv. See rust-lang/rust#75080 for the rationale; it's mostly irrelevant to clippy, but it does make the links easier to read and write which seems worth it in itself.

Please write a short comment explaining your change (or "none" for internal only changes)
changelog: none

@jyn514
Convert clippy_lints to intra-doc links
347d5ef 
This commit can (mostly) be replicated with `cargo intraconv`.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 1, 2020
flip1995
flip1995 approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Let's properly fix our documentation.

clippy_lints/src/mem_replace.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
clippy_lints/src/path_buf_push_overwrite.rs Outdated Show resolved Hide resolved
clippy_lints/src/types.rs Outdated Show resolved Hide resolved
clippy_lints/src/types.rs Outdated Show resolved Hide resolved
clippy_lints/src/types.rs Outdated Show resolved Hide resolved
clippy_lints/src/verbose_file_reads.rs Outdated Show resolved Hide resolved
@jyn514 @flip1995
Use backticks for code
f9254bb 
Co-authored-by: Philipp Krones <[email protected]>
@jyn514
Copy link
Member Author

jyn514 commented Dec 1, 2020 

diff of stderr:

-error: invalid suffix `x` for integer literal
+error: invalid suffix `x` for number literal
   --> $DIR/ice-3891.rs:2:5
    |
 LL |     1x;
    |     ^^ invalid suffix `x`
    |
-   = help: the suffix must be one of the integral types (`u32`, `isize`, etc)
+   = help: the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.)

not sure what I could have changed, sorry :/

@matthiaskrgr
Copy link
Member

That's changes from upstream rustc-repo-clippy that are waiting to be synced into this repo, see https://github.com/rust-lang/rust-clippy/pull/6404/files

But the sync is currently blocked by this ICE which happens in our tests rust-lang/rust#79560

flip1995
flip1995 reviewed Dec 2, 2020
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

I missed one thing

clippy_lints/src/functions.rs
@@ -212,7 +212,7 @@ declare_clippy_lint! {
/// ```
///
/// Note that there are crates that simplify creating the error type, e.g.
/// [`thiserror`](https://docs.rs/thiserror).
/// [`thiserror`].
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// [`thiserror`].
/// [`thiserror`](https://docs.rs/thiserror).

@nico-abram nico-abram mentioned this pull request Dec 2, 2020
Merged
phansch
phansch reviewed Dec 2, 2020
View reviewed changes
clippy_lints/src/types.rs
@@ -171,7 +171,7 @@ declare_clippy_lint! {

declare_clippy_lint! {
/// **What it does:** Checks for use of `&Box<T>` anywhere in the code.
/// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
/// Check the [`Box` documentation](std::boxed) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Will these links also work on https://rust-lang.github.io/rust-clippy/?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, how is that documentation built? If you're using mdbook then it won't work. I guess there's not much point to the PR then :/

Copy link
Member

Choose a reason for hiding this comment

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

mdbook? You wish! We're still using our good ol' python script. 😄

Copy link
Member

Choose a reason for hiding this comment

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

So yeah... I don't think we can change this in our lint documentation..

@jyn514
Copy link
Member Author

jyn514 commented Dec 2, 2020

:(

@jyn514 jyn514 closed this Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

@phansch phansch phansch left review comments

Assignees

@llogiq llogiq

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.
6 participants
@jyn514 @rust-highfive @matthiaskrgr @phansch @flip1995 @llogiq