-
Notifications
You must be signed in to change notification settings - Fork 103
ref: Fix clippy 1.63.0 lints #1411
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
Conversation
relay-common/src/constants.rs
Outdated
| // Note: This type is represented as a u8 in Snuba/Clickhouse, with Unknown being the default | ||
| // value. We use repr(u8) to statically validate that the trace status has 255 variants at most. | ||
| #[derive(Clone, Copy, Debug, PartialEq, Serialize)] | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of our schema types do not implement Eq for a reason, because that allows us to add floats without breaking equality. There's also absolutely no disadvantage to not implementing Eq in practice.
Ultimately, this comes down to API stability. I think we should disable this lint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i wasn't entirely sure about that lint but we don't really have a stable api at all so i don't care too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also absolutely no disadvantage to not implementing Eq in practice.
I was surprised about this lint as well. Will remove it.
clippy::derive_partial_eq_without_eq(see discussion below).#skip-changelog