Skip to content
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

chore: clean up dependencies #13728

Merged
merged 6 commits into from
Dec 13, 2024
Merged

chore: clean up dependencies #13728

merged 6 commits into from
Dec 13, 2024

Conversation

comphead
Copy link
Contributor

@comphead comphead commented Dec 10, 2024

Which issue does this PR close?

Related to #13726
Closes #.

Rationale for this change

Clean up dependencies
Going forward Im planning to add crate level hint #![warn(unused_extern_crates)] to specific crates, cannot do that globally as we got a lot of false positives on crates having test/bench sub crates.

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the development-process Related to development process of DataFusion label Dec 10, 2024
@comphead
Copy link
Contributor Author

If there are unused packages the clippy will fail

@@ -34,5 +34,5 @@ runs:
echo "RUSTC_WRAPPER=sccache" >> $GITHUB_ENV
echo "SCCACHE_GHA_ENABLED=true" >> $GITHUB_ENV
echo "RUST_BACKTRACE=1" >> $GITHUB_ENV
echo "RUSTFLAGS=-C debuginfo=line-tables-only -C incremental=false" >> $GITHUB_ENV
echo "RUSTFLAGS=-C debuginfo=line-tables-only -C incremental=false -Wunused-crate-dependencies" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

if this is global lint, it could be defined in cargo.toml in the lint section.
However, this is awesome for production code, but may be hard to do for tests/benchmark code.
At least in arrow-rs, i tried to enable this globally but failed.
see apache/arrow-rs#6804 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

going through the same. I'm keen to disable the lint for benches lets see where it can bring me

@github-actions github-actions bot added logical-expr Logical plan and expressions physical-expr Physical Expressions optimizer Optimizer rules core Core DataFusion crate common Related to common crate execution Related to the execution crate proto Related to proto crate and removed development-process Related to development process of DataFusion labels Dec 12, 2024
@comphead comphead changed the title CI: Warn on unused crates CI: Clean up dependencies Dec 13, 2024
@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Dec 13, 2024
@comphead
Copy link
Contributor Author

@findepi PTAL

serde_json = { workspace = true }
test-utils = { path = "../../test-utils" }
tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot", "fs"] }
tokio-postgres = "0.7.7"
Copy link
Member

Choose a reason for hiding this comment

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

beautiful

@findepi
Copy link
Member

findepi commented Dec 13, 2024

So this ended up with no CI changes for now, right?

It's a good change nonetheless. Just PR title may want a change.

@findepi findepi changed the title CI: Clean up dependencies chore: clean up dependencies Dec 13, 2024
@findepi findepi merged commit e8314ab into apache:main Dec 13, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Related to common crate core Core DataFusion crate execution Related to the execution crate logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Physical Expressions proto Related to proto crate sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants