BIND 10 #1866: isc.dns constants like RRType.A() should be constants, not functions
BIND 10 Development
do-not-reply at isc.org
Mon Feb 4 22:25:22 UTC 2013
#1866: isc.dns constants like RRType.A() should be constants, not functions
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner:
Type: defect | vorner
Priority: medium | Status:
Component: libdns++ | reviewing
Keywords: | Milestone:
Sensitive: 0 | Sprint-20130205
Sub-Project: DNS | Resolution:
Estimated Difficulty: 5 | CVSS Scoring:
Total Hours: 0 | Defect Severity: N/A
| Feature Depending on Ticket:
| Add Hours to Ticket: 0
| Internal?: 0
-------------------------------------+-------------------------------------
Comment (by jinmei):
Thanks for the review.
Replying to [comment:14 vorner]:
> First, would it make sense to put the script to convert into the
repository, let's say to the tools directory? In case some user already
used our libraries, it might be useful for them. We should, of course,
make it clear it is not supported in any way.
I don't necessarily oppose to that, but at this time I'd rather avoid
including mostly-unused file. Realistically, I suspect there are yet
very few people using this Python library for development other than
ourselves.
> I think the convention how to keep empty git directories is to place a
`.keep` file there, not `gitignore`. The `.gitignore` has special meaning
for git and it feels a bit like an abuse.
Okay, but I've actually removed the directory with the dot- file
based on your other comment below, so this point is now moot.
> The tests fail for me:
> {{{
> Running test: session_tests.py
> ...F..........F..........................
> ======================================================================
> FAIL: test_convert_rrset_class (__main__.SessionModuleTests)
> }}}
As I responded separately, I couldn't reproduce it, and I suspect it
was because your build environment is incomplete.
> It is not code from the branch, but this regexp seems to be wrong
(missing a dot after the backslash). It is in the generator script:
> {{{#!python
> elif re.search('\cc$', file):
> }}}
Ah, right, fixed.
> Why do you generate the code into a variable and accumulate it across
all the RRtypes and then dump it after each new one? This seems like a
quadratic run time and it looks unnecessary. Would it make sense to output
it just once at the end?
I guess you (at least) meant generate_typeclasscode(), where we built
the auto-generated code by extending a string object. I'm not sure if
the internal implementation is that inefficient, and even if so, if it
matters in this usage. But I revised the code to what I guess you
would have envisioned.
> Would it be more lightweight and easier to read if the list of „fake“
types (the ones that don't exist or we don't have them implemented yet)
would be by a single file with a list instead of many effectively empty
files?
Hmm, I once considered that approach and at that time it seemed to
require heavier updates to the generator script. But on looking at it
again it seems to be quite straightforward. So I adopted the idea
(but instead of introducing a separate file, simply listing them in
the generator script), removing the placeholder files.
--
Ticket URL: <http://bind10.isc.org/ticket/1866#comment:17>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list