
AMDG I've finished reading through all the documentation except for the reference. Here are the comments I have so far. Overall the documentation is quite good. I thought that the Boost.Parameter convention is to give keywords names starting with an _. I think that it would be a little nicer to work with attributes with an interface like this: BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id); fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message() That way, it's easier to guarantee static type safety and is also a little less verbose when accessing the attributes. Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary. I don't like the inconsistency of "sample_%N.log", vs. "[%TimeStamp%]: %_%" Could you use "sample_%N%.log"? Nevermind - I see that you're matching Boost.DateTime I would like logging::extract< unsigned int > to return a boost::optional or something like that instead of having an output parameter. Is it possible to declare and define a global logger separately? I think LineID may be a bad name, since when I saw it, I assumed it meant the line in the source that generated the log message, perhaps RecordID would be better? I don't like the inconsistency of logging::make_attr_ordering< unsigned int >("Line #"). I though the name was LineID? I don't like the use of a time in ordering_asynchronous_sink to specify how long to wait for a new record. It just seems too fragile. Suppose that the machine is running under a heavy load or I suspend the process, or put the machine to sleep? I don't like having the correctness depend on how long things take. If nothing else, it needs to be stated that ordering_asynchronous_sink is only a "best effort" mechanism and makes no guarantees. fmt::xml_dec. I would prefer a more intuitive name like xml_encode or xml_escape. (Since dec is "obviously" an abbreviation of decrement) In http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/sink_backend..., the documentation says that custom_level_mapping works for integral types, but the example uses std::string. Some of the code examples overflow the window. Why are there separate classes for handling the syslog and windows event log mappings? It seems like they're basically doing the same thing. Using scoped attributes to add attributes to a specific log record seems like a hack. I don't really like to see the lambda capabilities implemented from scratch. At the very least, I'd like to see plans for porting it to Phoenix v3 once it's ready. A full lambda library is far more feature complete than the minimal lambda that you provide. For instance satisfies, would be better as bind(f, attr<...>(...)); For string_literal, I seem to recall that Boost.Test already has a basic_cstring. Maybe this should be factored out instead of being duplicated. I wish that the names of functions and classes linked to the doxygen reference. You claim in http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/utilities.ht... that std::type_info is required to be unique for each type. I could not find such a requirement in the standard. Please provide the reference. Also, std::type_info has no member raw_name in the standard. If it exists, it's an extension. Why is type visitor::visit not spelled operator()? The stuff for extracting the values of attributes is so complex. Why can't you just have a single interface like visit<boost::mpl::vector<int, char, double> >(attr, f); rather than providing all these complicated layers. I guess that doesn't work when the set of possible types isn't known at compile time, but I still think it should be possible to cut down the number of redundant public interfaces. I'm not convinced that make_exception_handler belongs in this library. In the todo section I noticed a reference to #3278. This was fixed a while ago. In http://boost-log.sourceforge.net/libs/log/doc/html/log/extension.html under the minimalistic sink backend, the description of consume doesn't seem to match the code: "The record, as it was stated before, contains a set of attribute values (goes as the first argument of the function) and the message string (goes as the second one)." vs. void stat_collector::consume(record_type const& rec); What happens if... A filter or formatter tries to log something. In Christ, Steven Watanabe