BIND 10 #1483: "findAll" method for ZoneFinder

BIND 10 Development do-not-reply at isc.org
Mon Dec 19 15:10:39 UTC 2011


#1483: "findAll" method for ZoneFinder
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  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 jelte):

 * owner:  jelte => vorner


Comment:

 Replying to [comment:12 vorner]:
 > > 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?
 >

 hmm, perhaps we should consider raising an exception if a log message
 contains the wrong number of arguments. Alternatively or additionally,
 perhaps we should consider not doing indirect log calls (where the log
 message id is passed to a function that calls the log message) in the
 first place. The latter would certainly make a script that runs through
 the code and performs these checks a lot easier (I do not believe we can
 have both complete and sound checks for this by doing unit tests and
 checking log output).

 > > 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).
 >

 One risk of not 'fixing' them is that we may forget to document parts that
 do not have such a reference (which is the reason Jeremy wants this to be
 done, and I tend to agree with him here).

 Hmm, what about compromise, and give each parameter the description "See
 <other method>", or even "see other-parameter-of-other-method", if doxygen
 supports such a construct (doesn't solve the longer and repetetive one,
 although it does make the html output a bit nicer)?

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


More information about the bind10-tickets mailing list