Skip to content

fix: cargo warnings of import item #10196

Merged
waynexia merged 4 commits intoapache:mainfrom
waynexia:fix-test-warnings
Apr 24, 2024
Merged

fix: cargo warnings of import item #10196
waynexia merged 4 commits intoapache:mainfrom
waynexia:fix-test-warnings

Conversation

@waynexia
Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

I found there are many warnings in the project. It looks like we don't run "cargo check" for (a large number of) test modules.

The majority kinds are

  • duplicated import from super::* or xx::prelude::*
  • default imported items since rust edition 2021

What changes are included in this PR?

  • Fix the warnings
  • Deny unused_imports in Cargo.toml
  • Try to run check for all targets in workspace level.

Are these changes tested?

N/A

Are there any user-facing changes?

N/A

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@github-actions github-actions bot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate labels Apr 23, 2024

- name: Check workspace in debug mode
run: cargo check
run: cargo check --all-targets --workspace
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should cover test modules

Comment thread Cargo.toml
large_futures = "warn"

[workspace.lints.rust]
unused_imports = "deny"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Deny unused import warning. Unfortunately, cargo check doesn't support things like -D warning at present rust-lang/cargo#8424

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @waynexia -- looks like a great clean up to me

Screenshot 2024-04-23 at 1 49 37 PM 👏

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@waynexia waynexia merged commit 4edbdd7 into apache:main Apr 24, 2024
@waynexia waynexia deleted the fix-test-warnings branch April 24, 2024 06:32
ccciudatu pushed a commit to hstack/datafusion that referenced this pull request Apr 26, 2024
* fix: cargo warnings of import item

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* deny unused imports

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* allow util macro re-export

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* lift windows target feature gate to mod level

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

---------

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Changes to the physical-expr crates sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants