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

Ignore clippy lint warning for derive default #334

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

mimullin-bbry
Copy link
Contributor

Rust version 1.62 made it possible to use #[derive(Default)] (see: rust-lang/rust#94457)

As of rust version 1.68, the default clippy configuration will print a warning for manually impl Derive on enums. As such, users of libbpf-cargo will begin to experience clippy warnings when they migrate to version 1.68.

libbpf-cargo's minimum rust version is 1.58, thus the recommended method to remove the clippy warnings, is not yet an option for the project.

Add a #[allow(clippy::derivable_impls)], to suppress the clippy warning on all generated skeletons.
Add a TODO to remove this once the project moves to a minimum rust version of 1.62

Signed-off-by: Michael Mullin [email protected]

@mimullin-bbry
Copy link
Contributor Author

NOTE: The errors with clippy lint is unrelated to this change. A separate ticket/commit should be done for that.

@danielocfb
Copy link
Collaborator

NOTE: The errors with clippy lint is unrelated to this change. A separate ticket/commit should be done for that.

Yeah, I am working on that. It's caused by the new Rust release.

Copy link
Collaborator

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

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

Looks good to me, @mimullin-bbry . Thanks! I am just wondering, why did you decide not add the allow annotation to the generated definition/impl directly but applied it to pretty much everything generated?

@mimullin-bbry
Copy link
Contributor Author

I am just wondering, why did you decide not add the allow annotation to the generated definition/impl directly but applied it to pretty much everything generated?

Because I didn't even think of that solution. What you describe is much much better and I'll see if I can implement it instead.

Rust version 1.62 made it possible to use #[derive(Default)]
on enums.
(see: rust-lang/rust#94457)

As of rust version 1.68, the default clippy configuration
will print a warning for manually impl Derive on enums.
As such, users of libbpf-cargo will begin to experience clippy warnings
when they migrate to version 1.68.

libbpf-cargo's minimum rust version is 1.58, thus the recommended method
to remove the clippy warnings, is not yet an option for the project.

Add a #[allow(clippy::derivable_impls)], to suppress the clippy warning
on all generated skeletons.
Add a TODO to remove this once the project moves to a minimum rust
version of 1.62

Signed-off-by: Michael Mullin <[email protected]>
Copy link
Collaborator

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks again! I did not specifically test that change, but did not find other paths where we generate an enum definition.

@danielocfb danielocfb linked an issue Jan 26, 2023 that may be closed by this pull request
@danielocfb danielocfb merged commit 5553d9c into libbpf:master Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As of Rust v1.62, enums can now use #[derive(Default)]
2 participants