-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Enable and fix unused_qualifications
lint
#7715
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
I am not convinced that it improves code readability. I think that for some cases, it decreases it |
6c1be4d
to
1e7411f
Compare
@sylvestre I agree that it some cases it makes things less readable -- but in that case the more explicit namespaced identifier should be used throughout the file. Otherwise you have a confusing situation - the same ident is used with and without the namespace just a few lines apart. This lint does not force shorter idents, it forces them to be consistently used per file. |
GNU testsuite comparison:
|
13e4918
to
28f5b33
Compare
GNU testsuite comparison:
|
src/uu/chcon/src/fts.rs
Outdated
@@ -16,9 +16,9 @@ use crate::os_str_to_c_string; | |||
|
|||
#[derive(Debug)] | |||
pub(crate) struct FTS { | |||
fts: ptr::NonNull<fts_sys::FTS>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no a fan of this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sylvestre you had a few NonNull
directly used - i updated those
Improve code readability
39f893a
to
e796718
Compare
e796718
to
76b1b68
Compare
GNU testsuite comparison:
|
Improve code readability. What's even more important is that the same identifier is used identically everywhere in one file - i.e.
chrono::DateTime
would not be used with a fully-qualified name in one place, but with justDateTime
in another.