diff --git a/doc/src/BCMInstallTargets.rst b/doc/src/BCMInstallTargets.rst index 23615a8..2e52bab 100644 --- a/doc/src/BCMInstallTargets.rst +++ b/doc/src/BCMInstallTargets.rst @@ -11,6 +11,10 @@ bcm_install_targets This installs the targets specified. The directories will be installed according to GNUInstallDirs. It will also install a corresponding cmake package config(which can be found with ``find_package``) to link against the library targets. +.. It doesn't "install the targets specified" as much as it "generates + installation rules" which install the specified targets. The wording in + 'cmake --help-command install' is a bit more clear. + .. option:: TARGETS ... The name of the targets to install. diff --git a/doc/src/BCMPackage.rst b/doc/src/BCMPackage.rst index b2d1794..229f486 100644 --- a/doc/src/BCMPackage.rst +++ b/doc/src/BCMPackage.rst @@ -10,6 +10,11 @@ bcm_find_package This works the same as cmake's ``find_package`` except in addition, it will keep track of the each call to ``bcm_find_package`` in the project. Functions like ``bcm_package`` and ``bcm_boost_package`` will include these dependencies automatically in cmake's package config that gets generated. +.. I don't understand why 'bcm_find_package' needs to do anything other than + what the standard find_package does. Can't the targets exported by + 'find_package' carry all the "package config" information needed for '.pg' + file generation? + ----------- bcm_package ----------- @@ -18,10 +23,17 @@ bcm_package This setups a non-boost package in cmake. This is similar to ``bcm_boost_package`` but with extra flexibility as it does not have boost specific conventions. This function creates a library target that will be installed and exported. In addition, the target will be setup to auto-link for the tests. +.. "setup to auto-link for the tests". This is a bit unclear. Maybe say + "Subsequent uses of 'bcm_test' will automatically include this target as a + dependency" or something along these lines. + .. option:: The name of the package. This is both the name of the library target and the cmake package config that can be used with ``find_package()``. +.. This is inconsistent with 'bcm_boost_package' below. Why does Boost have to + be special? + .. option:: VERSION Sets the version of the package. @@ -30,6 +42,10 @@ Sets the version of the package. This will parse the version from a header file. It will parse the macros defined in the header as ``__VERSION_MAJOR``, ``__VERSION_MINOR``, and ``__VERSION_PATCH``. The ``prefix`` by default is the package name in all caps, but can be set using the ``VERSION_PREFIX`` option. +.. I'm a bit unsure about the above and following option. I think many codebases + use their own "version header" format and hardcoding this one is a but + unflexible. + .. option:: VERSION_PREFIX This sets the prefix that macros used to define the version will use. @@ -74,3 +90,5 @@ The source files to build the library. This specifies internal boost dependencies, that is, dependencies on other boost libraries. The libraries should not be prefixed with ``boost_`` nor ``boost::``. +.. Why not use 'boost::' here? This special casing will be unobvious for casual + readers of code which uses this library. diff --git a/doc/src/BCMProperties.rst b/doc/src/BCMProperties.rst index eb4a80b..8057db9 100644 --- a/doc/src/BCMProperties.rst +++ b/doc/src/BCMProperties.rst @@ -10,6 +10,9 @@ CXX_EXCEPTIONS This property can be used to enable or disable C++ exceptions. This can be applied at global, directory or target scope. At global scope this defaults to On. +.. Why these particular C++ options? If we go down this route, won't someone + always be dissappointed because their particular option won't be here? + -------- CXX_RTTI -------- diff --git a/doc/src/Intro.rst b/doc/src/Intro.rst index cf2a640..41dab82 100644 --- a/doc/src/Intro.rst +++ b/doc/src/Intro.rst @@ -5,6 +5,14 @@ Motivation This provides cmake modules that can be re-used by boost and other dependencies. It provides modules to reduce the boilerplate for installing, versioning, setting up package config, and creating tests. +.. It isn't immediately clear what this is going to provide for the user. + Perhaps something like this: BCM provides CMake functions for building and + releasing software packages. It is a simplification of CMake's builtin + functions and additionally incorporates pkg-config support. + +.. This section should be expanded as well. Perhaps with a side-by-side + comparison of the CMake way of doing things in comparison to the BCM way. + ===== Usage ===== @@ -44,6 +52,10 @@ The BCM modules provide some high-level cmake functions to take care of all the bcm_auto_pkgconfig() +.. The above snippet is mysterious for folks not accustomed to this library. + Where are the '.cpp' files being added? Where are the include files? What + do these functions do? + This sets up the Boost.Config cmake with the version ``1.61.0``. More importantly the user can now install the library, like this:: mkdir build @@ -63,6 +75,8 @@ Or if the user isn't using cmake, then ``pkg-config`` can be used instead:: g++ `pkg-config boost_config --cflags --libs` foo.cpp +.. Very nice! + ------------ Dependencies ------------ @@ -76,6 +90,12 @@ For dependencies on other boost libraries, they can be listed with the ``DEPENDS config ) +.. This is interesting. I like the reduction in boilerplate, but I question the + choice to go in such a different direction from the standard CMake way of + doing things. It strikes me as a possibly leaky abstraction where folks will + have a mix of code which uses both the CMake way and the BCM way; maintainers + will have to be familiar with both. + This will call ``find_package`` for the dependency and link it in the library. This structured this way to be able to support superbuilds(ie building all libraries together) in the future. ----- @@ -86,6 +106,11 @@ The BCM modules provide functions for creating tests that integrate into cmake's bcm_test(NAME config_test_c SOURCES config_test_c.c) +.. 'that' should be 'which' in the first sentence in the above paragraph I'm + pretty sure. + This will compile the ``SOURCES`` and run them. Also, there is no need to link in the ``boost_config``, as ``bcm_test`` will automatically link it in for us. Also, tests can be specified as compile-only or as expected to fail:: bcm_test(NAME test_thread_fail1 SOURCES threads/test_thread_fail1.cpp COMPILE_ONLY WILL_FAIL) + +.. Nice!