BIND 10 #558: Disable log4cxx dependency in logging API

[BIND 10 Development]

#558: Disable log4cxx dependency in logging API
-------------------------------------+-------------------------------------
                 Reporter:  stephen  |                Owner:  jelte
                     Type:  defect   |               Status:  reviewing
                 Priority:  major    |            Milestone:  R-Team-
                Component:  logging  |  Sprint-20110222
                 Keywords:           |           Resolution:
Estimated Number of Hours:  2.0      |            Sensitive:  0
                Billable?:  1        |  Add Hours to Ticket:  0
                Internal?:  0        |          Total Hours:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jelte


Comment:

 > I'm unsure what the goal (and/or the next step) is now; I had the
 following comments,
 > until I realized that the whole purpose was to really remove log4cxx
 use, but only
 > 'temporarily', as it says in the ticket, so what to do about this?
 >
 > lib/log/Makefile.am (also) contains a few commented-out includes (for
 LOG4CXX) that can
 > probably be removed now (as well as the commented-out part of
 configure.ac)
 >
 > is logger_impl_log4cxx.h still used (or cc for that matter)?

 I suggest that the redundant files (logger_impl_logcxx.* and
 xdebuglevel.*) are left in the distribution for now, until we decide what
 packages we're using (at the Prague meeting).  The file documentation.txt
 does mention them and notes how to get back to using log4cxx.

 > The pimpl use itself (in logger_impl.h) seems a bit off, isn't the
 > idea not to actually *define* the xxxImpl class in the headers, but
 > to only name it and let the .cc define it?

 As I understand it, pimpl means that the client code using the public API
 - in this case code using logger.h - is not exposed to the internals of
 the implementation.  How the implementation class is laid out - defined in
 the .cc file or in its own .cc file - is not part of pimpl.

 > is lib/log/tests/xdebuglevel_unittest still used? (it got commented out
 in the makefile)
 > (perhaps also related to above)

 No - it is related to the above.

 > Really minor (and i mean MINOR, one would almost call it trivial) naming
 point:
 > !LoggerImpl?* getLoggerptr() {
 > It's private and all, just wanted to mention that we tend to capitalize
 'ptr' in the rest of
 > our code :) (although, come to think of it, we use the Ptr postfix
 usually to name a shared_ptr)

 I'll change it - "Logger" and "Ptr" are two words so should be
 caiptalised.  It also occurs to me that I should disable the copy
 constructor and assignment operator.  (Copying loggers makes no sense -
 just create another one.)

 > You might want to have the generator include some comments before that
 initializer and
 > instantiator as to why they are necessary, and why they have such a
 weird name (and/or
 > why that doesn't matter).
 Done.

 > !LoggerImpl?, lots of lexical casts there (because of the change of
 MessageID I guess), which
 > seem unnecessary in this context, why not pass the ident and text as
 separate arguments to
 > output(), and have that function operator<<() them separately?
 Good catch, done.


 > I am not sure I oppose your suggestion at the bottom of #559, btw,
 perhaps more of this
 > actually warrants a focused phone discussion?
 OK, I'll organise something.

 > Apart from this, the code looks ok, btw :) (though perhaps Jinmei wants
 to have a quick
 > look at the changes for the init problem)
 I'll pass it to Jinmei for his comments.

-- 
Ticket URL: <http://bind10.isc.org/ticket/558#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development