Skip to content

Conversation

@nyurik
Copy link
Contributor

@nyurik nyurik commented Apr 9, 2025

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 just DateTime in another.

@sylvestre
Copy link
Contributor

I am not convinced that it improves code readability. I think that for some cases, it decreases it

@nyurik nyurik force-pushed the unused_qualifications branch from 6c1be4d to 1e7411f Compare April 9, 2025 16:54
@nyurik
Copy link
Contributor Author

nyurik commented Apr 9, 2025

@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.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@nyurik nyurik force-pushed the unused_qualifications branch 2 times, most recently from 13e4918 to 28f5b33 Compare April 9, 2025 20:43
@github-actions
Copy link

github-actions bot commented Apr 9, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)


#[derive(Debug)]
pub(crate) struct FTS {
fts: ptr::NonNull<fts_sys::FTS>,
Copy link
Contributor

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

Copy link
Contributor Author

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

@nyurik nyurik force-pushed the unused_qualifications branch 2 times, most recently from 39f893a to e796718 Compare April 10, 2025 20:59
@nyurik nyurik force-pushed the unused_qualifications branch from e796718 to 76b1b68 Compare April 10, 2025 21:13
@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre sylvestre merged commit 5bfbc30 into uutils:main Apr 10, 2025
69 checks passed
@nyurik nyurik deleted the unused_qualifications branch April 10, 2025 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants