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

Issue build error on warnings #213

Closed
GabLotus opened this issue Aug 16, 2020 · 6 comments
Closed

Issue build error on warnings #213

GabLotus opened this issue Aug 16, 2020 · 6 comments
Labels
A-Build-System Related to build systems or continuous integration D-Good-First-Issue Nice and easy! A great choice to get started with Bevy

Comments

@GabLotus
Copy link
Contributor

In the spirit of keeping the bevy codebase awesome, we should issue errors on warnings.

@GabLotus GabLotus added A-Build-System Related to build systems or continuous integration D-Good-First-Issue Nice and easy! A great choice to get started with Bevy labels Aug 16, 2020
@Ratysz
Copy link
Contributor

Ratysz commented Aug 16, 2020

Perhaps some warnings, not all of them? This alleviates the risk of new Rust version introducing new warnings and breaking builds of all client apps, until they downgrade their compiler or a new Bevy version addresses the new warnings.

One does not have to turn a yellow line into a red one to stop ignoring it.

@multun
Copy link
Contributor

multun commented Aug 16, 2020

That's unfortunately a huge no-no of building, for the reason mentionned above: the project should still build with newer compilers

@multun
Copy link
Contributor

multun commented Aug 16, 2020

There's a PR to add clippy to the CI, which ensures code with compiler and linter warnings can't be merged

@karroffel
Copy link
Contributor

I think the idea is to do this only on CI, so users or even developers will be unaffected. This would only concern checking in code into this repository (which then needs to be made sure compiles on the used toolchain).

This might go well together with a MSRV commitment where Rust version updates are considered breaking changes (to the MSRV at least) and might need fixes for the new warnings too, but again would not affect users.

@coltontcrowe
Copy link

There's a PR to add clippy to the CI, which ensures code with compiler and linter warnings can't be merged

Here's the PR mentioned above: #206

@Moxinilian
Copy link
Member

Implemented in #206

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 D-Good-First-Issue Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

No branches or pull requests

6 participants