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.

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

move suspicious_doc_comments to doc pass #11798

Merged
merged 1 commit into from Nov 13, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Nov 13, 2023

This was my first lint. I've been meaning to move it over to doc.rs since that's a better place.
There weren't any changes made to the lint logic itself.

I guess this can be considered part of #11493

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2023

r? @giraffate

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 13, 2023
@flip1995
Copy link
Member

The doc.rs file is now almost 1kLoC. It might make sense to split it up into a doc/ module. This doesn't really have anything to do with this PR. Just some food for thought.

@flip1995
Copy link
Member

@bors r+

While I'm already here, I went ahead and reviewed the PR. Thanks for cleaning this up!

@bors
Copy link
Collaborator

bors commented Nov 13, 2023

📌 Commit d89890d has been approved by flip1995

It is now in the queue for this repository.

@y21
Copy link
Member Author

y21 commented Nov 13, 2023

@flip1995 That's a good idea. Same for attrs.rs probably? It's in a similar situation with 1k+ LOC

@bors
Copy link
Collaborator

bors commented Nov 13, 2023

⌛ Testing commit d89890d with merge a4b2864...

@flip1995
Copy link
Member

attrs.rs also makes sense, looking at it. 👍

@bors
Copy link
Collaborator

bors commented Nov 13, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing a4b2864 to master...

@bors bors merged commit a4b2864 into rust-lang:master Nov 13, 2023
5 checks passed
bors added a commit that referenced this pull request Nov 20, 2023
Split `doc.rs` up into a subdirectory

So, first, sorry for the bad diff. 😅

In #11798, `@flip1995`  suggested splitting `doc.rs` up, much like how we have the `methods/`, `matches/`, `types/` subdirectories.
I agree with this, the file is getting bigger as we add more and more doc lints that it makes sense to do this refactoring.

This is purely an internal change that moves things around a bit.
(**EDIT:** depending on the outcome of #11801 (comment) , this may change the lint group name from `doc_markdoc` to `doc`).

I tried to not change any of the actual logic of the lints and as such some things weren't as easy to move to a separate file. So we still have some `span_lint*` calls in the `doc/mod.rs` file, which I think is fine. This is also the case in `methods/mod.rs`.

Also worth mentioning that the lints missing_errors_doc, missing_panics_doc, missing_safety_doc and unnecessary_safety_doc have a lot of the same logic so it didn't make much sense for each of these to be in their own file. Instead I just put them all in `missing_headers.rs`

I also added a bit of documentation to the involved `check_{attrs,doc}` methods.

changelog: none
bors added a commit that referenced this pull request Nov 20, 2023
Split `doc.rs` up into a subdirectory

So, first, sorry for the bad diff. 😅

In #11798, `@flip1995`  suggested splitting `doc.rs` up, much like how we have the `methods/`, `matches/`, `types/` subdirectories.
I agree with this, the file is getting bigger as we add more and more doc lints that it makes sense to do this refactoring.

This is purely an internal change that moves things around a bit.
(**EDIT:** depending on the outcome of #11801 (comment) , this may change the lint group name from `doc_markdoc` to `doc`).

I tried to not change any of the actual logic of the lints and as such some things weren't as easy to move to a separate file. So we still have some `span_lint*` calls in the `doc/mod.rs` file, which I think is fine. This is also the case in `methods/mod.rs`.

Also worth mentioning that the lints missing_errors_doc, missing_panics_doc, missing_safety_doc and unnecessary_safety_doc have a lot of the same logic so it didn't make much sense for each of these to be in their own file. Instead I just put them all in `missing_headers.rs`

I also added a bit of documentation to the involved `check_{attrs,doc}` methods.

changelog: none
bors added a commit that referenced this pull request Nov 20, 2023
Split `doc.rs` up into a subdirectory

So, first, sorry for the bad diff. 😅

In #11798, `@flip1995`  suggested splitting `doc.rs` up, much like how we have the `methods/`, `matches/`, `types/` subdirectories.
I agree with this, the file is getting bigger as we add more and more doc lints that it makes sense to do this refactoring.

This is purely an internal change that moves things around a bit.
(**EDIT:** depending on the outcome of #11801 (comment) , this may change the lint group name from `doc_markdoc` to `doc`).

I tried to not change any of the actual logic of the lints and as such some things weren't as easy to move to a separate file. So we still have some `span_lint*` calls in the `doc/mod.rs` file, which I think is fine. This is also the case in `methods/mod.rs`.

Also worth mentioning that the lints missing_errors_doc, missing_panics_doc, missing_safety_doc and unnecessary_safety_doc have a lot of the same logic so it didn't make much sense for each of these to be in their own file. Instead I just put them all in `missing_headers.rs`

I also added a bit of documentation to the involved `check_{attrs,doc}` methods.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants