BIND 10 #517: Empty node processing in MemoryZone difficult Part

BIND 10 Development do-not-reply at isc.org
Tue Jan 25 01:42:36 UTC 2011


#517: Empty node processing in MemoryZone difficult Part
-------------------------------------+-------------------------------------
                 Reporter:  hanfeng  |                Owner:  jinmei
                     Type:           |               Status:  reviewing
  enhancement                        |            Milestone:  A-Team-
                 Priority:  major    |  Sprint-20110126
                Component:  data     |           Resolution:
  source                             |            Sensitive:  0
                 Keywords:           |  Add Hours to Ticket:  0
Estimated Number of Hours:  0.0      |          Total Hours:  0
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I've taken a quick look at it, and made some minor editorial changes I'd
 propose (2f151e4).

 gcov indicated all code is covered by tests, which is good.

 Most of the editorial changes are s/foo */foo*/, s/foo &/foo&/, etc.
 This seems to be quite common patches you submitted.  If this convention
 is difficult for you to follow, I'd suggest you write a simple checker
 script and run it (and fix any trivial style issues) before you ask a
 review.  That would help the reviewer focus on the more substantial part
 of code and make the review process smoothier.

 One non-editorial change is that for the RBTree class constructor.
 I changed needsReturnEmptyNode_ to a const variable (so that we
 don't have to worry about the case where it's dynamically modified),
 and initialize the variables in a member initialization list
 (as that's the only way to initialize a const variable).

 Another observation.  Some of the newly written documentation doesn't seem
 to be sufficient.  In general, we should provide for each method/class
  - one-or-two-line brief description
  - more description that normally consists of several paragraphs.
    it includes various design choices or usage notes, and in case of
    methods possible exceptions and if it's not exception free which level
    of exception guarantee is provided.
  - for methods params and return

 There can be exceptions (e.g. for a very trivial method like a trivial
 accessor), but normally if some of the above points are missing the
 documentation is considered poor (or at least I'd consider it poor as a
 reviewer:-)  Please check if the new doc provides sufficient information
 according to these points, and fix them as I review the rest of the code.

 Review continues.

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


More information about the bind10-tickets mailing list