Skip to content

Move unnecessary_unsafety_doc to pedantic #9989

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

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

xFrednet
Copy link
Member

This lint was added in #9822. I like the idea, but also agree with #9986 as well. I think it should at least not be warn-by-default. This is one of these cases, where I'd like a group between pedantic and restriction. But I believe that users using #![warn(clippy::pedantic)] will know how to enable the lint if they disagree with it.


Since the lint is new:

changelog: none

r? @flip1995 since I'd suggest back porting this, the original PR was merged 16 days ago.

Closes: #9986 (While it doesn't address everything, I believe that this is the best compromise)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 29, 2022
@xFrednet xFrednet added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 29, 2022
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 agree that this lint should not be warn-by-default. I wouldn't even put it in pedantic though. I don't see a point in suggesting to remove documentation. If people want to have Safety comments on only unsafe functions, they should enable this lint explicitly IMO.

So I would put it in restriction.


If we merge this before tomorrow, I don't think any backport will be necessary. We had a >4 week sync drought. I will make sure that I'm right though, so I'm keeping the label until I've done that.

@xFrednet xFrednet force-pushed the 9986-move-safety-thingy branch from 6754b8d to cc43c3d Compare November 30, 2022 11:59
@xFrednet
Copy link
Member Author

So I would put it in restriction.

I was considering that, but held off on it since it seems some wanted it in style. pendantic seemed to be the middle. I've now moved it to restriction

If we merge this before tomorrow, I don't think any backport will be necessary. We had a >4 week sync drought. I will make sure that I'm right though, so I'm keeping the label until I've done that.

Even better 🎉

@flip1995
Copy link
Member

There was no discussion about the lint group in the PR, so I don't think there was a compelling reason for putting it in style.

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Nov 30, 2022

📌 Commit cc43c3d has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 30, 2022

⌛ Testing commit cc43c3d with merge 78589ff...

@bors
Copy link
Contributor

bors commented Nov 30, 2022

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

@bors bors merged commit 78589ff into rust-lang:master Nov 30, 2022
@xFrednet xFrednet deleted the 9986-move-safety-thingy branch November 30, 2022 13:02
@flip1995
Copy link
Member

flip1995 commented Dec 8, 2022

Just checked: This lint is not in beta yet and on the current rust-lang/rust master it is already in restriction. So no need to backport anything. Thanks for pointing this out anyway!

@flip1995 flip1995 removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 8, 2022
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.

unnecessary_safety_doc considered harmful
4 participants