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