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

Minor: fixup contribution guide about clippy #5952

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Minor: fixup contribution guide about clippy #5952

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,23 +128,16 @@ cargo +stable fmt --all -- --check

## Clippy Lints

We recommend using `clippy` for checking lints during development. While we do not yet enforce `clippy` checks, we recommend not introducing new `clippy` errors or warnings.
We use `clippy` for checking lints during development, and CI runs `clippy` checks.

Run the following to check for `clippy` lints:

```bash
# run clippy with default settings
cargo clippy
cargo clippy --workspace --all-targets --all-features -- -D warnings
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work on my archlinux x86_64:

error: sub-expression diverges
   --> arrow-flight/tests/flight_sql_client_cli.rs:657:9
    |
657 |         unimplemented!("Implement do_action_end_savepoint")
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
    = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info)

error: sub-expression diverges
   --> arrow-flight/tests/flight_sql_client_cli.rs:665:9
    |
665 |         unimplemented!("Implement do_action_cancel_query")
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
    = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `arrow-flight` (test "flight_sql_client_cli") due to 7 previous errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That looks like it may be due to the fact you are using nightly rust by default

does it work with:

cargo +stable clippy --workspace --all-targets --all-features -- -D warnings


```

More comprehensive `clippy` checks can be run by adding flags:

```bash
# run clippy on the arrow crate with all features enabled, targeting all tests, examples, and benchmarks
cargo clippy -p arrow --all-features --all-targets
```

If you use Visual Studio Code with the `rust-analyzer` plugin, you can enable `clippy` to run each time you save a file. See https://users.rust-lang.org/t/how-to-use-clippy-in-vs-code-with-rust-analyzer/41881.

One of the concerns with `clippy` is that it often produces a lot of false positives, or that some recommendations may hurt readability. We do not have a policy of which lints are ignored, but if you disagree with a `clippy` lint, you may disable the lint and briefly justify it.
Expand Down
Loading