BIND 10 #1483: "findAll" method for ZoneFinder

[BIND 10 Development]

#1483: "findAll" method for ZoneFinder
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111220
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:  DDNS   |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:11 jelte]:
 > Not related to this changeset specifically, but any idea what that
 boolean 'keep_doing' is, er, doing in Query::process()? This looks like
 some variable and associated loop that isn't actually used anymore. (and
 hence it looks like both can be removed)

 Yes, you're right, the variable is useless there. It got removed, because
 it obviously didn't keep doing whatever it was supposed to.

 > The new log messages DATASRC_DATABASE_FOUND_ANY and
 DATASRC_DATABASE_WILDCARD_ANY are not called with the number of arguments
 they expect.

 This is a general problem of logAndCreateResult ‒ it seems like none of
 the messages logged through that uses all the parameters, I used it the
 same way as it is used everywhere else. Unfortunately, it complains
 slightly in the log output.

 So, how do you think we should fix it? Remove the complaints from the
 logging? Or ensure somehow only the relevant parts are supplied to the
 message?

 > If we want to have doxygen produce zero warnings at some point (i
 believe Jeremy had this intent, so that we can then enable a test for that
 as well), we should not add more. So if so, I think we should have doxygen
 comments for the parameters and return values of the new methods as well
 (note that we can keep the reference to find(), and they can be quite
 simple)

 This is exactly one of the places where I think doxygen warnings are
 broken. „Fixing“ this warning would not only be useless, as it doesn't
 give the user any new information. It would be actually wrong, because:
  * It makes the documentation both longer and repetitive, making it harder
 to read.
  * The parameters are handled by the same code. With code, we try very
 hard not to have some twice, copy-pasted code is regarded wrong because it
 is hard to follow, and when one copy is fixed, the others need too, but
 nobody really knows where ever the copies are. The same problem applies to
 the copy-pasted documentation, it tends to grow inconsistent over time.

 I believe the actual documentation quality is more important than having
 no warnings, that's why I wouldn't like to „fix“ it. But it is possible I
 don't see some other way than copy-pasting and having it twice would be
 actually helpful for user (I don't see how, though).

 Thanks

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