Skip to content

Add configuration to lint missing docs of pub(crate) items #10303

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
Merged
bors merged 7 commits into from
Feb 21, 2023
Merged

Add configuration to lint missing docs of pub(crate) items #10303

bors merged 7 commits into from
Feb 21, 2023

Conversation

@pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Feb 6, 2023 •
edited
Loading

Fixes this: #5736 (comment)

TODO:

  • Needs docs
  • Needs better names
  • Should pub items be checked to when this new option is enabled? I'm saying no because missing_docs already exists

@flip1995 I'd like to get some input from you :)

changelog: Enhancement: [missing_docs_in_private_items]: Added new configuration missing-docs-in-crate-items to lint on items visible within the current crate. For example, pub(crate) items.
#10303

@rustbot
Copy link
Collaborator

rustbot commented Feb 6, 2023

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

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 6, 2023
@pvdrz pvdrz marked this pull request as ready for review February 9, 2023 21:42
@pvdrz pvdrz mentioned this pull request Feb 9, 2023
Merged
@flip1995
Copy link
Member

flip1995 commented Feb 10, 2023

  • Should pub items be checked to when this new option is enabled? I'm saying no because missing_docs already exists

Agreed. I also am fine with the name. I'll let @giraffate do the final review, I only skimmed over the name and documentation.

giraffate
giraffate reviewed Feb 15, 2023
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Thanks! I asked one question.

The tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stdout file is empty, so it should be removed.

@pvdrz pvdrz requested a review from giraffate February 16, 2023 16:46
giraffate
giraffate approved these changes Feb 20, 2023
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Overall looks good, thanks!

I made small comments. I'll merge this once those are addressed.

@pvdrz
Copy link
Contributor Author

pvdrz commented Feb 20, 2023

Thanks @giraffate! It seems we're good to go then

@giraffate
Copy link
Contributor

giraffate commented Feb 20, 2023

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Feb 20, 2023

📌 Commit 790f28b has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 20, 2023

⌛ Testing commit 790f28b with merge b1cf1e7...

@bors
Copy link
Contributor

bors commented Feb 21, 2023

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

@bors bors merged commit b1cf1e7 into rust-lang:master Feb 21, 2023
@bors bors mentioned this pull request Feb 21, 2023
Merged
@pvdrz pvdrz deleted the pub_crate_missing_docs branch February 21, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@giraffate giraffate giraffate approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@giraffate giraffate

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.

5 participants

@pvdrz @rustbot @flip1995 @giraffate @bors