Skip to content

Conversation

@yew1eb
Copy link
Contributor

@yew1eb yew1eb commented Jan 18, 2026

Which issue does this PR close?

Closes #1911

Rationale for this change

What changes are included in this PR?

  1. Enable -D warnings for Clippy in CI, and set workspace.lints = true for all sub-crates to align with unified workspace lint config.
  2. Resolve auto-fixable lint issues via cargo clippy --fix.
  3. Manually exempt existing panic-related code with the #[allow(clippy::panic)] annotation

Are there any user-facing changes?

How was this patch tested?

Copy link
Contributor

@ShreyeshArangath ShreyeshArangath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables stricter Clippy linting in the CI pipeline by switching from allowing all warnings to denying all warnings by default, while selectively allowing specific lints that require more extensive refactoring to fix. The PR also resolves many auto-fixable clippy warnings throughout the codebase and annotates existing panic-related code with #[allow(clippy::panic)] attributes.

Changes:

  • Configure workspace-level lints with unwrap_used and panic denied, while temporarily allowing other lints that need gradual refactoring
  • Update CI and build scripts to use cargo clippy --all-targets --workspace -- -D warnings instead of selectively denying only unwrap_used
  • Apply auto-fixes for numerous clippy lints including needless_borrow, redundant_closure, needless_return, useless_format, uninlined_format_args, manual_range_contains, into_iter_on_ref, and others
  • Add #[allow(clippy::panic)] annotations to existing panic-heavy code with comments indicating future refactoring plans

Reviewed changes

Copilot reviewed 56 out of 56 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Cargo.toml Add workspace-level lint configuration denying unwrap_used and panic, while temporarily allowing other lints
dev/mvn-build-helper/build-native.sh Update clippy command to deny all warnings instead of only unwrap_used
.github/workflows/tpcds-reusable.yml Update clippy command in CI workflow to deny all warnings
native-engine/*/Cargo.toml Add [lints] workspace = true to all sub-crates
native-engine/datafusion-ext-plans/src/**/*.rs Add panic allow annotations to existing panic-heavy code
native-engine/datafusion-ext-functions/src/**/*.rs Apply auto-fixes for clippy lints and add panic annotations
native-engine/datafusion-ext-exprs/src/**/*.rs Apply auto-fixes for needless borrows, redundant closures, and other clippy suggestions
native-engine/datafusion-ext-commons/src/**/*.rs Apply extensive auto-fixes including format strings, redundant references, and test improvements
native-engine/auron*/src/**/*.rs Apply auto-fixes for format strings, needless returns, and const thread_local initialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable -D warnings and resolve related issues

3 participants