search

bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35k stars 3.43k forks source link

Validate that benches and tools crates are formatted in CI #10768

Open alice-i-cecile opened 9 months ago

alice-i-cecile commented 9 months ago 
          Yep. This is what we have to do, otherwise someone else is gonna run into this eventually.

Additionally, from the Bevy side, this should be checked by CI.

Originally posted by @rlidwka in https://github.com/bevyengine/bevy/pull/10758#pullrequestreview-1749866027

matiqo15 commented 9 months ago

The issue here lies in that benches are not checked by cargo fmt --all -- --check. I've found two ways to solve it:

  1. We can add "benches" to members of workspace in Cargo.toml. However, this could add additional overhead - especially as "benches" are currently explicitly excluded from members.
  2. We could add flag in tools/ci/src/main.rs. This would then be: cmd!(sh, "cargo fmt --all --manifest-path ../../../benches/Cargo.toml -- --check"). While not best, it seems to be the only solution I've found if we don't want to add benches to workspace.
alice-i-cecile commented 9 months ago

I think we should start with solution 2 for now.

mockersf commented 9 months ago

As you said, the issue is that benches (and compile fail crates) are excluded from the workspace: https://github.com/bevyengine/bevy/blob/4221f7e7e9ae5b2d664a067f1fbabc97a5806232/Cargo.toml#L15-L21

I don't remember why, but if we want the benches to be checked in CI (format, clippy, ...) they should be in the workspace.