refactor: fix clippy issues for near-o11y #8291
Conversation
|
IMO we should decide which clippy lints we're enabling first, set them to In particular I would probably advocate for disabling the lint that necessitated the Another approach that I have been using in the past is set up clippy without lints at all, and then enable them one-by-one, fixing the specific lints for the entire codebase at once. It then makes discussions about whether the lint is valuable for us or not much more… substantiated. |
|
I would advocate allowing all style category lints for the initial pass, actually. No reason to hinder the adoption with what are somewhat opinionated calls. Lets give time for people to get used to having clippy tell them about important stuff first. I think they’ll be much more open to majority of the style & pedantic lints once they know clippy is a highly valuable tool :) |
|
Very good points, I like the idea of @nagisa to allow all style category lints to start with. Adding them one-by-one and discussing whether a particular lint is useful seems like a good approach. How about we allow all and warn/deny on |
core/o11y/benches/metrics.rs
Outdated
| @@ -63,7 +63,7 @@ fn inc_counter_vec_with_label_values_stack_no_format(bench: &mut Bencher) { | |||
| loop { | |||
| idx -= 1; | |||
| buf[idx] = b'0' + (n % 10) as u8; | |||
| n = n / 10; | |||
| n /= 10; | |||
There was a problem hiding this comment.
this also feels a bit odd to me, eitherway it's fine but personally I don't think n /= 10 is better than n = n / 10
|
@nagisa @jakmeier thanks for sharing your feedback on the overall approach. I agree that it makes sense to first enable clippy with the most useful lints ( I will merge with this PR while reverting the |
This enables clippy `clippy::correctness` checks as part of CI as discussed [here](#8291 (comment)). It is added as part of `sanity check` step for consistency, since that is where `cargo check` is executed. This fixes the following issues in order to pass the added checks: * noop `.clone()` * this loop never actually loops * `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `self.burst == 0` instead This PR is a part of #8145
This enables clippy `clippy::correctness` checks as part of CI as discussed [here](near#8291 (comment)). It is added as part of `sanity check` step for consistency, since that is where `cargo check` is executed. This fixes the following issues in order to pass the added checks: * noop `.clone()` * this loop never actually loops * `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `self.burst == 0` instead This PR is a part of near#8145
Fix the issues reported by
cargo clippy -p near-o11yas part of #8145