-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Adjust dead code lint to account for fields that implement Drop, see … #29439
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
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (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. 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. |
@bors r+ rollup Looks good to me, thanks for the PR! |
📌 Commit 75b6768 has been approved by |
@Manishearth #21775 (comment) suggests something different from what this patch does... |
@bors r- Oh, I forgot about that. |
@dagit in this case you should move the check to be a span_lint_note somewhere here. |
Trying to make sure I understand correctly. So instead of removing the warning for Drop fields, you want the warning to suggest adding a |
Yes |
Updates? |
@Manishearth Sorry, I haven't been able to work on it much since last week. Work is crunch time this week. I hope to have time after that. Given that the correct way of doing this is unrelated to the way I did it previously, should I revert those changes and continue to use this PR or create a new one? It seems like the git history would be cleaner if I created a new one, but I know that in the typical case it's better to reuse the existing PR. Thanks. |
No problem. A new PR works too. It's possible to update this old PR by force pushing to the branch (so the git history stays clean), but not necessary |
What's the status of this PR? |
…#21775
r? @Manishearth