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

Use clippy in CI? #158

Open
lopopolo opened this issue May 21, 2023 · 2 comments
Open

Use clippy in CI? #158

lopopolo opened this issue May 21, 2023 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@lopopolo
Copy link
Contributor

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
Copy link
Owner

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.

@BurntSushi BurntSushi added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels May 21, 2023
@lopopolo
Copy link
Contributor Author

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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants