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

Fix clippy complaints #6573

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Conversation

itsjunetime
Copy link
Contributor

Which issue does this PR close?

None

Rationale for this change

When running cargo clippy on this project with the latest nightly (and even with stable), it raises a lot of issues. This just goes through each of those issues and fixes them.

What changes are included in this PR?

Small changes to fix the issues clippy was complaining about. Most of this is removing explicit lifetimes where unnecessary, but there are also a few instances of fixing doc comments and other small things.

Are there any user-facing changes?

Some doc comments that previously wouldn't be appearing correctly in docs.rs will now show up where expected.

@github-actions github-actions bot added parquet Changes to the parquet crate arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate labels Oct 16, 2024
@@ -15,32 +15,32 @@
// specific language governing permissions and limitations
// under the License.

//! Rle/Bit-Packing Hybrid Encoding
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @itsjunetime -- most of these changes look like very nice improvements 👌

I think we just need to get CI passing and it will be good to go

@@ -336,7 +336,7 @@ impl FFI_ArrowSchema {
Ok(HashMap::new())
} else {
let mut pos = 0;
let buffer: *const u8 = self.metadata as *const u8;
let buffer: *const u8 = self.metadata;
Copy link
Contributor

Choose a reason for hiding this comment

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

this appears to not compile on CI 🤔

e.g. https://github.com/apache/arrow-rs/actions/runs/11370592216/job/31636347622?pr=6573

Run cargo test -p arrow-data --all-features
   Compiling bitflags v2.6.0
   Compiling arrow-schema v53.1.0 (/__w/arrow-rs/arrow-rs/arrow-schema)
error[E0308]: mismatched types
   --> arrow-schema/src/ffi.rs:339:3[7](https://github.com/apache/arrow-rs/actions/runs/11370592216/job/31636347622?pr=6573#step:6:8)
    |
339 |             let buffer: *const u[8](https://github.com/apache/arrow-rs/actions/runs/11370592216/job/31636347622?pr=6573#step:6:9) = self.metadata;
    |                         ---------   ^^^^^^^^^^^^^ expected `*const u8`, found `*const i8`
    |                         |
    |                         expected due to this
    |
    = note: expected raw pointer `*const u8`
               found raw pointer `*const i8`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `arrow-schema` (lib) due to 1 previous error
Error: Process completed with exit code [10](https://github.com/apache/arrow-rs/actions/runs/11370592216/job/31636347622?pr=6573#step:6:11)1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just fixed - clippy was complaining about casting to the same type, but I had foolishly forgotten what I'd just learned about c_char = (i8|u8) so I removed the cast. It's now a cast with an #[allow()]

@@ -26,7 +26,7 @@ pub fn round_upto_multiple_of_64(num: usize) -> usize {
/// Returns the nearest multiple of `factor` that is `>=` than `num`. Here `factor` must
/// be a power of 2.
pub fn round_upto_power_of_2(num: usize, factor: usize) -> usize {
debug_assert!(factor > 0 && (factor & (factor - 1)) == 0);
debug_assert!(factor > 0 && factor.is_power_of_two());
Copy link
Contributor

@Dandandan Dandandan Oct 17, 2024

Choose a reason for hiding this comment

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

Very nice

@itsjunetime itsjunetime force-pushed the june/fix_clippy_complaints branch from 2fa0ead to 773997b Compare October 18, 2024 19:14
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @itsjunetime -- this looks great to me

also thank you @Dandandan and @mbrobbel for the review

@alamb
Copy link
Contributor

alamb commented Oct 21, 2024

I merged up from master to hopefully get a clean CI run

@tustvold tustvold merged commit 2043353 into apache:master Oct 21, 2024
28 checks passed
@alamb
Copy link
Contributor

alamb commented Oct 21, 2024

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants