-
Notifications
You must be signed in to change notification settings - Fork 713
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
Fix all clippy lints in parquet crate #1254
Comments
I'd like to have a try if no one else has been doing it! |
There are many dead code and unused public functions. I am not sure whether to clean them up. |
I recommend doing this issue in a few PRs -- in the first take care of the easier lints, and then in follow ons we can sort out dead code / public function nonsense together |
Lots of warning: enum is never used: `LevelDecoder`
--> parquet/src/encodings/levels.rs:151:10
|
151 | pub enum LevelDecoder {
| ^^^^^^^^^^^^
warning: associated function is never used: `v1`
--> parquet/src/encodings/levels.rs:165:12
|
165 | pub fn v1(encoding: Encoding, max_level: i16) -> Self {
| ^^
warning: associated function is never used: `v2`
--> parquet/src/encodings/levels.rs:180:12
|
180 | pub fn v2(max_level: i16) -> Self {
| ^^
warning: associated function is never used: `set_data`
--> parquet/src/encodings/levels.rs:194:12
|
194 | pub fn set_data(&mut self, num_buffered_values: usize, data: ByteBufferPtr) -> usize {
| ^^^^^^^^
warning: associated function is never used: `set_data_range`
--> parquet/src/encodings/levels.rs:221:12
|
221 | pub fn set_data_range(
| ^^^^^^^^^^^^^^
warning: associated function is never used: `is_data_set`
--> parquet/src/encodings/levels.rs:242:12
|
242 | pub fn is_data_set(&self) -> bool {
| ^^^^^^^^^^^
warning: associated function is never used: `get`
--> parquet/src/encodings/levels.rs:254:12
|
254 | pub fn get(&mut self, buffer: &mut [i16]) -> Result<usize> {
| ^^^
warning: associated function is never used: `buffer`
--> parquet/src/encodings/rle.rs:171:12
|
171 | pub fn buffer(&self) -> &[u8] {
| ^^^^^^
warning: associated function is never used: `get`
--> parquet/src/encodings/rle.rs:358:12
|
358 | pub fn get<T: FromBytes>(&mut self) -> Result<Option<T>> { |
Sorry for the late reply @HaoYang670
If the code is dead I think we should remove it. One way for I am not sure how you are running clippy, but it may also be that some of these structs are only used when certain features of the I believe our CI system has good coverage so feel free to try removing them and if the CI passes I think it is a good change |
Thank you @alamb. There are some experimental mods under |
Clippy is run like this: https://github.com/apache/arrow-rs/blob/master/.github/workflows/rust.yml#L211-L250
Perhaps we can also add the |
After #2190 clippy is run just for the parquet crate (which now includes the So once that is merged, the first order of business will be to make this command run cleanly: cargo clippy -p parquet --all-targets --all-features -- -D warnings And then we can move on to removing the additional lints |
Describe the bug
Due to "historical reasons" there are several clippy lints that are disabled in the parquet crate
https://github.com/apache/arrow-rs/blob/master/parquet/src/lib.rs#L18-L36
It would be great to clean up the code to pass these lints for tidiness
To Reproduce
Remove one of the
#[allow]
lines above, runclippy
Expected behavior
Clippy runs cleanly without blank
#allow
across the whole crateAdditional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: