I like having fmt support. The way the optional dependency is handled in the header seems a bit esoteric though - the fmt_format.hpp header has the following:
#if __has_include(<fmt/format.h>) && __has_include(<fmt/base.h>)
And then gets included in <boost/decimal.hpp>. By convention, headers
with optional dependencies don't get included in the convenience header (e.g. see Asio with OpenSSL). As a user, I'd prefer a "can't find include <fmt/format.h>" error than "X function is not defined"
when the dependency can't be found for whatever reason.
Fair enough. I already have fmt separated out in the case that you consume Decimal as a module so that's an easy change.
Some points:
* BOOST_DECIMAL_REDUCE_TEST_DEPTH, debug_pattern and bit_string are still defined in public headers, when they belong to tests. * The example on literals [3] hasn't updated the using namespace clause (should be boost::decimal::literals rather than boost::decimal). You might find it useful to build and run the examples on CI, and include the code in the docs by using asciidoc tagged regions [4]. * boost::decimal::to_string seems to still be public but not documented. * The docs list an entry for Rounding Mode that looks like a dead link [5]. * I'd try to avoid using the <boost/decimal.hpp> include at all in the
examples. We then get complaints on "Boost makes your builds slow". Specially, please try to avoid this recommendation in the tutorial [6]. * The examples section topics are great, but the content seems more like unit tests than examples [7]. I miss text explaining what the example is trying to do, some comments, etc. * The example on charconv uses an assert to check the return value, and this makes me very uncomfortable [8]. I've found real-life projects with people "checking" return values with assert like this (there are developers that haven't heard about NDEBUG, apparently). So I'd be more comfortable with an "if (r_from) { /* handle failure */ }". * There are still places marked by LCOV_EXCL_START/LCOV_EXCL_STOP that correspond to valid code paths. Please try to avoid this. It's okay not having 100% coverage, but marking corner cases as "won't happen" makes the metric less reliable.
Thank you for your detailed points and review. They all seem sound to me. Matt