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