chore: enforce clippy::allow_attributes for common crates#18988
Conversation
| // $(#[allow(deprecated)])? | ||
| { | ||
| $(let value = $transform(value);)? // Apply transformation if specified | ||
| #[allow(deprecated)] |
There was a problem hiding this comment.
Hmm I wonder if there was a historical reason these allow deprecations were present; was it anticipating future compatibility? 🤔
There was a problem hiding this comment.
it may have been needed for code that was subsequently updated but the allow(deprecated) annotation was not. That is why I like the expect(deprecated) style as then the compiler will tell you when it is no longer actually needed
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
…utes-for-common-crates' into chore-enforce-clippyallow_attributes-for-common-crates
chakkk309
force-pushed
the
chore-enforce-clippyallow_attributes-for-common-crates
branch
from
980020f to
9ad4fe0
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks like a nice improvement to me -- thank you @chakkk309 and @Jefffrey
| { | ||
| // Ok to use spawn here as SpawnedTask handles aborting/cancelling the task on Drop | ||
| #[allow(clippy::disallowed_methods)] | ||
| #[expect(clippy::disallowed_methods)] |
| // $(#[allow(deprecated)])? | ||
| { | ||
| $(let value = $transform(value);)? // Apply transformation if specified | ||
| #[allow(deprecated)] |
There was a problem hiding this comment.
it may have been needed for code that was subsequently updated but the allow(deprecated) annotation was not. That is why I like the expect(deprecated) style as then the compiler will tell you when it is no longer actually needed
chakkk309
deleted the
chore-enforce-clippyallow_attributes-for-common-crates
branch
## Which issue does this PR close? Part of apache#18881 ## Rationale for this change Implement clippy::allow_attributes lint for **datafusion-common** and **datafusion-common-runtime** crates ## What changes are included in this PR? - Add #![deny(clippy::allow_attributes)] to both crates - Convert #[allow(...)] to #[expect(...)] attributes - Remove unnecessary lint suppressions discovered through unfulfilled expectations ## Are these changes tested? yes ## Are there any user-facing changes? No user-facing changes. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
3 participants
Which issue does this PR close?
Part of #18881
Rationale for this change
Implement clippy::allow_attributes lint for datafusion-common and datafusion-common-runtime crates
What changes are included in this PR?
Are these changes tested?
yes
Are there any user-facing changes?
No user-facing changes.