-
Notifications
You must be signed in to change notification settings - Fork 13.4k
changes lint name: ctypes -> ffi_rust_types #15277
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
Can you squash these commits? |
While I filed the issue, and believe this is a 'better' name, there are some other considerations here:
Since I've been the most vocal about this, I'd love to hear others' opinions. |
I belive ctypes is the only lint with this problem? I went through the list, and the others seem fine. The only other one which is borderline is So I don't think this is a wider problem, unless you're refering to something else. I do think this is worth fixing, though. For a transition plan, that's a good point. We could keep in support for ctypes but rather than doing the normal linty thing, check if it is used and emit a deprecation warning saying to use the new name. |
As far as deprecation goes and involving more lints goes, we could keep a map of aliases for different names for each lint (with a 'preferred' name) and if the preferred name is not the one used we could print the deprecation warning. Support for the aliases could be dropped in a later version once appropriate. My only question of this is where this alias map would live. Thoughts on that? @brson which other lints have this problem? If we use the map as I suggested we could easily add support for other lints which have this problem which for which an alias would be nice so that a user typing #{allow[ctypes]} wouldn't be shocked that they were given an error due to a non-existent lint I also agree with @cmr in that another, more explicit name would probably be better. I'm not sure, however, that the name should necessarily be longer, as I don't think doing bad things should necessarily be a pain to do. If anything, I would argue it be shorter so the user is less focused on things like getting the spelling correct and more focused on making sure they're writing safe code. |
Closing due to inactivity, but feel free to reopen with a rebase! |
So I understand why it's good to close this RFC, it is very out of date from master. However, I don't understand why, 4 weeks later, my questions were left unanswered. I realize that everybody is busy, but having no comments on an RFC for 4 weeks can be a bit aggravating. |
We try to ensure that the queue keeps moving, but sadly sometimes things fall through the cracks. It's not necessarily anyone's responsibility to ensure that all PRs get proper care and attention, but contributors should always feel free to ping PRs to send out notifications with requests for review every so often to remind reviewers. I tend to just go through every so often and close PRs that haven't had any activity for more than 10 days, and you really should feel free to reopen with a rebase! |
limit `change_visibility` assist to applicable items this pr limits the `change_visibility` assist to applicable items. top level items in this context means items that are not nested within `fn`s or `trait`s. now ```rs fn foo { // assists on this `struct` keyword won't include `change_visibility` struct Bar {} } trait Foo { // same with the `fn` here fn bar(); } ```
fixes #14704