enumflags2 icon indicating copy to clipboard operation
enumflags2 copied to clipboard

Lint fixes

Open chris-ha458 opened this issue 2 years ago • 2 comments

Some small fixes regarding formatting

chris-ha458 avatar Oct 06 '23 01:10 chris-ha458

Is this guided by the output of any particular linter? Also, CI fails on the MSRV run.

meithecatte avatar Oct 06 '23 11:10 meithecatte

Mostly from different levels of clippy output. I wasn't aware of the msrv requirement here. I'll take a look.

chris-ha458 avatar Oct 06 '23 13:10 chris-ha458

I have decided that the changes here range from barely any improvement, though meaningless, to actively making the code harder to read, and that it isn't worth my time to separate the minuscule improvements from the rest.

I have noticed that you have a habit of making such drive-by PRs to various projects. Please consider that you're essentially generating noise, using up the energy of the maintainers for matters which are utterly trivial.

It's not that I haven't noticed that I've written if !something { ... } else { ... }. The reason this lint is off by default is that sometimes it makes sense to do so. The condition uses a negation, but it's basically asking "is this the normal case". It makes sense to handle it in that order!

Insisting on using then_some is also questionable here. It got added to the standard library to make method chaining work in more situations, but here that advantage goes out the window because a prefix negation is needed. And then you need to remember what then_some does! The one good thing it does is that this line now sticks out like a sore thumb and makes you ask "hang on, what's going on here, why do we need this". And indeed, it would probably make sense to refactor that variable out of existence. But that would require thought and judgement, and not just the mindless application of a linter.

Please ask yourself what the purpose of what you're doing is. If you send readability fixes for code you're not otherwise working on, they better be significant. This is not significant.

meithecatte avatar Feb 12 '24 06:02 meithecatte