BIND 10 #1003: root logger names

[BIND 10 Development]

#1003: root logger names
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  UnAssigned
                       Type:  task   |                Status:  reviewing
                   Priority:  major  |             Milestone:
                  Component:         |  Sprint-20110802
  logging                            |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  Core
Feature Depending on Ticket:         |  Estimated Difficulty:  2.0
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I made a few trivial changes (basically editorial/style related)
 and pushed them directly to the branch.

 '''ccsession.cc'''

 - b10Prefix(): I don't see the need for cast.  According to
   http://www.cplusplus.com/reference/clibrary/cctype/tolower/
   "The value is returned as an int value that can be implicitly casted
   to char."

 - This comment seems to be a verbatim copy of the ticket description,
   and doesn't 100% match what the function specifically does:
 {{{
 +// This function prefixes the name read in the configuration with 'b10-"
 and
 +// leaves the module code as it is. [...]
 }}}
   that is, "this function...leaves the module code as it is" doesn't
   make sense.

 - This comment doesn't make sense to me:
 {{{
 +    // since we'll only be updating one first-level element,
 +    // and we return as const again, a shallow map copy is
 +    // enough
 }}}
   What does 'return as const' mean?  Perhaps you intended to make the
   return value of this function ConstElementPtr?

 - This doesn't parse:
 {{{
 +// Copies the map for a logger, changing the of the logger.
 }}}
   "changing the <something> of the logger"?

 - Is it safe to call LOG_xxx in getRelatedLoggers().  I don't follow
   all possible control flows, but this seems to be producing a log
   message while configuring logging, which looks fragile.

 '''config_messages.mes'''

 - CONFIG_LOG_EXPLICIT: s/will updated/will be updated/?
 {{{
 +configuration for the program will updated with the information.
 }}}
   same for CONFIG_LOG_WILD_MATCH.

 '''ccsession_unittests.cc'''

 - why adding iostream?  The other changes to this file don't seem to
   require it (at least not directly)
 - CCSessionTest() constructor: not a big deal, but I'd make root_name
   const and initialize it in the initialization list.
 - I'd test the case where the input name is "b10-test".
 {{{
     doRelatedLoggersTest("[ { \"name\": \"b10-test\" }]",
                          "[]");
 }}}

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