Minor: chores: Update clippy in pre-commit.sh #8810
Conversation
There was a problem hiding this comment.
Thanks @my-vegetable-has-exploded -- I think this is better than main and so moves things forward. I left a suggestion to maybe make it even better
| @@ -60,7 +60,7 @@ echo -e "$(GREEN INFO): cargo clippy ..." | |||
|
|
|||
| # Cargo clippy always return exit code 0, and `tee` doesn't work. | |||
| # So let's just run cargo clippy. | |||
| cargo clippy | |||
| cargo clippy --all-targets --workspace --features avro,pyarrow -- -D warnings | |||
There was a problem hiding this comment.
I wonder if it would be better to run the same exact script CI does, namely
https://github.com/apache/arrow-datafusion/blob/main/ci/scripts/rust_clippy.sh
(the CI runs it here https://github.com/apache/arrow-datafusion/blob/e86253919983ffe89d74ec1310f5f080482adcff/.github/workflows/rust.yml#L475C14-L475C39)
There was a problem hiding this comment.
I tried to just use source ./ci/scripts/rust_clippy.sh here, but with set -ex in it, the process would terminate if clippy is not happy which causes subsequent fmt won't execute.
So I copy the whole cargo command.
|
Thanks again @my-vegetable-has-exploded |
|
Thanks @alamb. |
Which issue does this PR close?
Rationale for this change
Sometimes my codes pass checks of pre-commit, but ci still fails. So I am wondering whether keeping the "clippy" consistent between in pre-commit.sh and in CI would be beneficial.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?