KDAB / KDDockWidgets

KDAB's Dock Widget Framework for Qt
https://www.kdab.com/development-resources/qt-tools/kddockwidgets
Other
754 stars 163 forks source link

CMake: option() is only for booleans #434

Closed ferdnyc closed 9 months ago

ferdnyc commented 9 months ago

The CMake option() command only supports boolean flags. All other configuration should be done using CACHE variables.

Breaking this rule causes GUI configuration tools like cmake-gui to display confusing/broken options when generating the build system, as any variable created with option() will be represented by a checkbox and cannot be set to arbitrary string values.

This PR migrates KDDockWidgets_FRONTENDS from an option() to a set(... CACHE STRING) variable, with the default value being the empty string (same as before).

The variable description is also expanded to note that leaving the field blank will enable autodetection. the help comment at the top of the root CMakeLists.txt is clarified by explicitly noting that it will autodetect Qt frontends.

iamsergio commented 9 months ago

thank you, have you signed the CLA already ? see https://kdab.github.io/KDDockWidgets/involved.html thanks

ferdnyc commented 9 months ago

have you signed the CLA already ?

I submitted one for GammaRay, back before it was switched over to the bot... so that should still be on file, if that's sufficient. Otherwise, no, not specifically for KDDockWidgets.

CLAassistant commented 9 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: ferdnyc
:x: pre-commit-ci[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

ferdnyc commented 9 months ago

Done! As for pre-commit, I had an open issue about that at https://github.com/KDAB/GammaRay/issues/814, where I wrote:

According to the README at https://github.com/cla-assistant/cla-assistant, bot accounts can be whitelisted for the CLA checks:

Can I allow bot user contributions?

Since there's no way for bot users (such as Dependabot or Greenkeeper) to sign a CLA, you may want to allow their contributions without it. You can do so by importing their names (in this case dependabot[bot] and greenkeeper[bot]) in the CLA assistant dashboard.

And the API datadump for my PR https://github.com/KDAB/GammaRay/pull/794 shows that the bot's login name is "pre-commit-ci[bot]".

iamsergio commented 9 months ago

I've squashed the pre-commit stuff into yours

It's been merged, thanks!

commit 0a58a56c5066d9f6fe01f1cb3498bfb71f8981e6 (HEAD -> 2.0, origin/2.0)
Author: FeRD (Frank Dana) <ferdnyc@gmail.com>
Date:   Wed Nov 15 11:25:55 2023 -0500

    CMake: option() is only for booleans

    The CMake option() command only supports boolean flags.[1] All other
    configuration should be done using CACHE variables.
    Breaking this rule causes GUI configuration tools like cmake-gui
    to display confusing/broken options when generating the build system.

    [1]: https://cmake.org/cmake/help/v3.28/command/option.html