BIND 10 #1484: python binding for ZoneFinder.findAll

[BIND 10 Development]

#1484: python binding for ZoneFinder.findAll
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  minor  |  Sprint-20120110
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:  DDNS   |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:14 vorner]:

 > > - ZoneFinder_helper_all: what's this?
 > > {{{#!c++
 > >     return Py_BuildValue("I", 1);
 > > }}}
 > >   This seems to be unreachable code (and more minor, it would need
 > >   parentheses for the return value per coding guideline).  This is
 > >   probably a mere copy of ZoneFinder_helper, and the same comment
 > >   applies to it.
 >
 > Yes, it's copy-paste from the ZoneFinder_helper. I don't know the true
 purpose of it, but I guess some of our build machines got confused and
 thought it could reach the end of the function without calling return, so
 it was put there to prevent the warning. I could remove it and see if the
 build bots trigger it or not, if we wanted to try the theory out.

 Okay, in that case I suggest removing it (them) for now.  I remember
 cases where such an otherwise-unnecessary return is needed as a
 workaround for a broken compiler if the code above it terminates with
 throw, but it's not the case for these two functions.

 If it fails on some buildbot we can re-add them with a comment of why
 we need them.

 > {{{
 > [func]*               vorner
 > The target parameter of ZoneFinder.find is no longer present, as the
 > interface was awkward. To get all the RRsets of a single domain, use
 > the new findAll method (the same applies to python version, the method
 > is named find_all).
 > }}}

 `ZoneFinder.find` would better be `ZoneFinder::find` as this sentence
 seems to talk about the C++ version.

 Other changes look okay.  If you agree on removing the unnecessary
 return (for now), please do so and merge.

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