openbmc / bmcweb

A do everything Redfish, KVM, GUI, and DBus webserver for OpenBMC
Apache License 2.0
154 stars 131 forks source link

Stop using #ifdef to enable/disable code #182

Closed edtanous closed 2 months ago

edtanous commented 3 years ago

In general, features are enabled or disabled using ifdef option flags. While useful if you need to change dependencies (like enabling/disabling ssl), in practice, most (possibly all) of the options are simply enabling or disabling code paths.

We've seen cases where code under those options will break silently when those options are used. Therefore, we should.

  1. Move all option configs into the config.h. Regardless of this problem, this would be good to do anyway to keep all the options code in one place. Today it's bifurcated.
  2. Change all the #ifdef FEATURE calls to use the runtime equivalent, if(x). If separate compilation is truly required, c++17 added if constexpr, which solves this.

Doing this should stop the pattern where code passes the CI build, but fails the per-machine build, or at least make it much less likely. It also ensures that all of our options, even the ones not used, remain buildable for everyone.

edtanous commented 2 months ago

Resolved by 25b54dba775b31021a3a4677eb79e9771bcb97f7