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.

Sign up for GitHub

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

Jump to bottom

Apply clippy fixes for Rust 1.83 #13596

Merged
merged 4 commits into from
Nov 29, 2024
Merged

Apply clippy fixes for Rust 1.83 #13596

merged 4 commits into from
Nov 29, 2024

Conversation

findepi
Copy link
Member

@findepi findepi commented Nov 28, 2024 •
edited
Loading

dev/rust_lint.sh no longer passes for me, maybe because of rustup update. This is first portion of fixes suggested by clippy.

@github-actions github-actions bot added sql SQL Planner physical-expr Physical Expressions optimizer Optimizer rules core Core DataFusion crate common Related to common crate functions labels Nov 28, 2024
@findepi findepi force-pushed the findepi/apply-clippy-fixes-e2c260 branch from cecf231 to 95acf32 Compare November 28, 2024 18:55
@github-actions github-actions bot added logical-expr Logical plan and expressions execution Related to the execution crate labels Nov 28, 2024
@findepi findepi changed the title Apply clippy fixes Apply clippy fixes for Rust 1.83 Nov 28, 2024
@github-actions github-actions bot added the catalog Related to the catalog crate label Nov 28, 2024
comphead
comphead reviewed Nov 28, 2024
View reviewed changes
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @findepi
Please help with the comments

@comphead
Copy link
Contributor

I think we need to #allow[missing_docs] as the clippy now complains on that

@findepi
Copy link
Member Author

findepi commented Nov 28, 2024

I think we need to #allow[missing_docs] as the clippy now complains on that

:) just added

@findepi
Copy link
Member Author

findepi commented Nov 28, 2024

now that #13598 is merged, let be rebase, otherwise we no longer test with 1.83

findepi and others added 4 commits November 28, 2024 21:41
@findepi
Apply clippy fixes
f1287ef 
`dev/rust_lint.sh` no longer passes for me, maybe because of `rustup
update`. This is first portion of fixes suggested by clippy.
@Dandandan @comphead @findepi
Fix typo
d984ae2 
Co-authored-by: Oleks V <[email protected]>
@findepi
Suppress missing docs clippy check in test code
0c8d919
@findepi
Revert "Temporarily pin toolchain version to avoid clippy (apache#13598…
d0a5e5f 
…)"

This reverts commit 2b37018.
Toolchain pinning is no longer needed.
@findepi findepi force-pushed the findepi/apply-clippy-fixes-e2c260 branch from fee0d90 to d0a5e5f Compare November 28, 2024 20:41
@findepi
Copy link
Member Author

findepi commented Nov 28, 2024

clippy job passed with toolchain pin dropped.

comphead
comphead approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@comphead comphead 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 @findepi

@jonahgao jonahgao merged commit 703b10d into apache:main Nov 29, 2024
26 checks passed
@jonahgao
Copy link
Member

Thanks @findepi @comphead @Dandandan

@findepi findepi deleted the findepi/apply-clippy-fixes-e2c260 branch November 29, 2024 08:15
@alamb alamb mentioned this pull request Feb 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dandandan Dandandan Dandandan left review comments

@comphead comphead comphead approved these changes

Assignees
No one assigned
Labels
catalog Related to the catalog crate common Related to common crate core Core DataFusion crate execution Related to the execution crate functions logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Physical Expressions sql SQL Planner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix build issues on latest stable Rust toolchain (1.83)
4 participants
@findepi @comphead @jonahgao @Dandandan