Re: [boost] [logging] Review of proposed logging library

From: John Torjo (john.groups_at_[hidden])
Date: 2008-02-12 11:35:53

Hi Arnstein ,

Thanks for the review.
> - configuration from configuration file(s) [NOT OK]
>   
True, but we're pretty close ;)
> - logger hierarchies with property/config propagation [NOT OK]
>   
If you mean logger hierarchies like in v1, they're not there. I realized 
that they are not as useful as I thought.
I will allow to copy one log into another. Do you think that's enough, 
or do you have a specific case where you'd like that?
> - possibility to use printf syntax when logging [NOK OK]
>
>   
Note - basically you can create your own gather class (which for 
instance can use boost::format) and implement this
> * What is your evaluation of the design?
>
> The major parts missing are logger hierarchies with support for
> configuration propagation and configuration of these hierarchies from
> config files. This is a feature supported by the C++ equivalents of log4j.
>
> The design in terms of classes and files seems fine by me. I like that
> the library is headers only. The use of macros will always be a
> discussion point, but it is convenient for a logging library. I do not
> have big issues with this. The separation of loggers, filters,
> formatters and writers appeal to me. Splitting the headers into forward
> declarations and definitions is very good to reduce compile time for big
> projects.
>
>   
Thanks :)
> * What is your evaluation of the documentation?
> Thorough documentation with a lot of examples and tutorials. I was able
> to find what I was looking for most of the time. Maybe a bit of
> reorganizing is needed because the front page seems a bit overwhelming
> at first. Formalization of some sentences  and comments would be nice if
> the library gets accepted into boost.
>
>   
Yes will do - others have noticed that as well.
>
> I also did a quick spin with valgrind (3.3.0):
> valgrind --tool=memcheck --leak-check=full ./mul_levels_one_logger
>
> Valgrind reports a lot of conditional jumps/moves based upon
> uninitialized values and usage of uninitialized variables. This has to
> be fixed.
>
>   
I will have to check this - once I learn how to use valgrind ;)
> * Do you think the library should be accepted as a Boost library?
> Yes, but compiler and valgrind warnings has to be fixed before
> submission. Also all the tests must be verified to run without errors on
> all supported platforms.
>
> A big thanks to John for putting the effort into making a useful logging
> library.
>
>   
Thanks ;)

Best,
John

-- 
http://John.Torjo.com -- C++ expert
http://blog.torjo.com
... call me only if you want things done right