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

Add clippy support and fix all warnings / errors #206

Merged
merged 6 commits into from
Aug 20, 2020

Conversation

multun
Copy link
Contributor

@multun multun commented Aug 16, 2020

Depends on #204 and #201

@karroffel karroffel added the A-Build-System Related to build systems or continuous integration label Aug 16, 2020
@cart
Copy link
Member

cart commented Aug 16, 2020

This one will take a bit more time for me to review. I havent used clippy much in practice so I need to sort out what I want from it.

@cart
Copy link
Member

cart commented Aug 16, 2020

to determine the urgency of this pr: were these changes all automated or did you need manual intervention? if its automated im fine waiting, but i dont want to throw away a bunch of manual work / make it hard to update this pr when we merge new stuff.

@multun
Copy link
Contributor Author

multun commented Aug 16, 2020

clippy can't automatically fix the vast majority of suggestions by itself yet, so I had to do it by hand.
Here's my take on clippy:

  • it makes the code more readable the vast majority of the time
  • it catches bugs (dfbdeeb and e36b26c)

It isn't perfect, but it doesn't let the kind of code one writes at 4am through

@cart
Copy link
Member

cart commented Aug 20, 2020

I learned a few new patterns from this pr. Nice!

This looks good to go.

@cart cart merged commit e31f576 into bevyengine:master Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants