Re: [boost] [log] Boost Logging Lib v2, 0.9.3

From: Jens Seidel (jensseidel_at_[hidden])
Date: 2007-10-31 09:52:47

Hi John,

after applying the attached patch, Boost.Log (including examples) should
be in a good shape, at least related to compiler errors and warnings.

On Wed, Oct 31, 2007 at 11:15:28AM +0200, John Torjo wrote:
> Fixed the rest also. Committed to SVN.

Thanks. I noticed that you also nearly always increment a version number
in the commit log:
  v0.9.10, 31 oct 2007
There should be no need for this or do you really release a new version
multiple times per day? Refering to the revision number is maybe better
especially as applying a single patch is often not sufficent to fix a
bug properly.

A few comments:

  * Why do you have compiler specific directories such as
    lib/logging/samples/gcc/ especially as files in it are
    (nearly) empty?
    I suggest to drop all kind of platform and compiler specific
    code from the test and sample files.
  * Some code evaluates the UNICODE macro to decide whether to use
    char or wchar. Why? Probably UNICODE is just a wrong name for
    something which should be called USE_MULTI_BYTE or similar.
    Unicode is a character set which exist in various encodings.
    Some encodings such as UTF-8 do not require wchar support at
    all!
  * In boost/logging/format/destination/defaults.hpp you use
    Windows related code to write into a "debug stream" and do nothing
    on other systems (this raised a warning about code without effect
    which you added trying to fix a "unused variable" warning). This
    contradicts with the documentation on other systems so that I
    changed it to write to std::cout in this case.
  * Some files are called stdafx.h but documentation refers to either
    STDAFX.H or StdAfx.h. Every proper operating system is able to use
    an arbritrary file system (Linux supports at least 30!). Does this
    mean this is not true for your proprietary system? I thought I read
    about (optional) support for ext2 or reiserfs filesystems on it as
    well (which are case sensitive).
    Why do you not drop stdafx.h. Is it really used?

Jens