BIND 10 #899: Implement logging using Log4cplus

[BIND 10 Development]

#899: Implement logging using Log4cplus
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  stephen
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110531
                  Component:         |            Resolution:
  logging                            |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
Feature Depending on Ticket:         |           Total Hours:  0
  logging                            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => stephen


Comment:

 Hello

 The patch is rather long one. But it is true it's quite simple, so I think
 it is mostly OK, with some details:
  * Why is the destructor of `DerivedLogger` explicitly mentioned and
 virtual? As this class has no virtual functions (except for the inherited
 ones), there's no need for one, the default will be virtual, because the
 destructor of parent class is already virtual. Not that it would be wrong,
 it just surprised me.
  * I don't like including the linker flags everywhere. Is it really
 needed? Because I think the whole purpose of our liblog is to hide the
 internal implementation.
  * This bit looks strange:
  {{{
 +
 +/// Minimum/maximum debug levels.  These are defined wi
 +
  }}}
  (in `logger_level.h`)
  * Isn't a class with only static methods a little bit misleading? Such
 class is more like namespace, isn't it? (Not that it would produce
 different code, in the end)
  * specify exact directory of log4cplus library and headers ‒ this message
 looks strange to me. Is it possible to specify inexact directory? And it
 looks like they should live in one directory, but one is in lib, one in
 include, I really wondered which one of them you mean for a short while.
 Wouldn't „installation prefix of log4cplus“ be better?
  * This FIXME in `Formatter::arg` was wrong, because the underlying
 replace did replace all occurrences. However, what you describe isn't the
 biggest problem (that is, actually, quite expected output). If we use the
 original example, you would now get "42 42" (the first %1 gets replaced to
 %2 and then both %2 get replaced), which is by no means expected output
 (it is understandable once you get it, but it's surprising).

 Also, as this brings a new dependency, I believe we need a changelog and a
 heads-up email for developers to install it if they didn't already.

 Thanks

-- 
Ticket URL: <https://bind10.isc.org/ticket/899#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development