bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust

https://bevyengine.org

Apache License 2.0

36.46k stars 3.6k forks source link

remove the detailed_trace macro #16450

Open mockersf opened 1 week ago

mockersf commented 1 week ago

Objective

Fix CI
Contribute to #11478

Solution

Remove the detailed_trace macro, and replace its use by the trace macro
The macro doesn't work: it doesn't check for the detailed_trace feature
If it checked for the feature, as the macro is expanded where it's used, the feature should be added to all crates
Log levels are already controllable by features

alice-i-cecile commented 1 week ago

@mockersf you're missing some imports, as CI helpfully points out: https://github.com/bevyengine/bevy/actions/runs/11940939339/job/33284703598?pr=16450#step:6:749

rparrett commented 1 week ago

Log levels are already controllable by features

Just linking those here for visibility: https://docs.rs/tracing/latest/tracing/level_filters/index.html#compile-time-filters

cart commented 1 week ago

Actually hold on. Can we just fix the macro for 0.15? Maybe that would be better than rushing this in. It would be good to have a full cycle to evaluate the performance implications of this.

mockersf commented 1 week ago

In that case, I would prefer to remove those logs completely.

They were not possible to enable through the feature, so no one should miss them

cart commented 1 week ago

I think we should discuss that as the permanent solution (ex: evaluate each log for value / do performance cost-benefit / decide the correct long term path for this category of thing). Imo we should minimally fix this for 0.15 without rocking the boat. It feels like removing logs indiscriminately is rocking the boat (even if they don't currently have users).