BurntSushi / bstr

A string type for Rust that is not required to be valid UTF-8.
Other
745 stars 51 forks source link

Use clippy in CI? #158

Open lopopolo opened 1 year ago

lopopolo commented 1 year ago

Hi @BurntSushi. I figured I'd open an issue to ask before putting up a PR. Are you interested in running clippy on this crate as part of CI?

Clippy reports 44 warnings and 2 hard errors on bstr currently. At a first glance, most of the warnings looked like good simplifications of either the std APIs called (e.g. swapping s.get(0) for s.first()) or the code itself (removing unnecessary lifetime annotations, using for x in iter instead of while let Some(x) = iter.next(), removing needless borrows, etc.).

The hard errors both revolve around deriving Hash when manually implementing PartialEq.

If you're open to it, I can put up a PR that fixes each clippy warning type in an isolated commit so you can see which lints you like and which ones you don't. I know for my own crates, I like being able to tailor clippy to my tastes.

BurntSushi commented 1 year ago

If you're open to it, I can put up a PR that fixes each clippy warning type in an isolated commit so you can see which lints you like and which ones you don't. I know for my own crates, I like being able to tailor clippy to my tastes.

If you're willing to do this, then I'm willing to give a try.

I've generally avoided Clippy because I do tend to disagree with a number of things it flags. And as a result, I find it too much trouble to work with. I know I can change the lint lists, and if I had one or two Rust projects, I think it would make sense to invest time in it. But I have dozens, and managing Clippy lint lists across all of them is, to me, a fair bit of work.

So I'm willing to give this a try, but if it ends up annoying me I may end up removing it in the future.

lopopolo commented 1 year ago

If you're willing to do this, then I'm willing to give a try.

sweet! I'm happy to take a stab at this. I've got a PR up here whenever you have a chance to take a look: