directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Enrique Rodriguez" <enriqu...@gmail.com>
Subject Re: Proposed protocol-dns changes
Date Mon, 29 Jan 2007 05:32:51 GMT
I agree with most of your changes and I intend to apply the patch once
we work out the test failure issue (binary PDU's missing).  It's good
to see activity on the DNS protocol.  My synopsys of your patch was
that it contained good updates to JDK 1.5 and a couple nice bug fixes
plus new features around decoding, required for DNS clients.

I thinks it's good practice to break a patch up into a couple smaller
patches.  This patch broke in one place for me and there was the
missing test files issue.  I would have been able to apply parts
faster.  The patch naturally breaks down into sub-patches, which you
were probably aware of as they are in order.  The order also
corresponds nicely to increasing complexity/invasiveness and
increasing broken-ness.  In short, the first 3 subpatches would have
gone on relatively smoothly, leaving us to deal with the tests.

1-2)  enums, type-safe wrappers (JDK 1.5 updates)
3-6)  codec reorg w.r.t. MINA deps
7-8)  bug fixes
9-11)  test cases

The same concept goes for large emails, which makes it harder to find
time to comment.

Other comments:

...
> 6) Moved all the (get|put)Unsigned(Byte|Short|Int) methods into a
> utility class called ByteBufferUtil.

Regarding #6, in the early days of MINA we found we had a need for
unsigned values in various protocols so we put them directly on MINA's
ByteBuffer:

http://mina.apache.org/report/trunk/apidocs/org/apache/mina/common/ByteBuffer.html

So, this may be a reason to use MINA ByteBuffer's.  I also don't see
any reason to reduce our dependency on MINA.  Especially with more
complicated protocols, there is a need to use features of the
framework to reduce complexity.  If the framework is truly pluggable,
then it isn't a framework, it's a component.

I'll comment on the non-patch commentary in a separate email.

Enrique

Mime
View raw message