On 5/21/10 6:17 AM, Felix Knecht wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Thanks for heads up as well.
(Just one thing, Felix : it would be more than appreciated if you could
run the full test suite before committing, as some parts of the code is
not necessarily as easy to fix as it seems. I'm not blaming you for
that, I'm just trying to get the process to run more smoothly.)
I thought that "mvn clean install -Dintegration" will do this (and I try
to run it before I commit the stuff, maybe I missed it on the latest
commit, sorry).
It normally does... I don't why I got this Stack Overflow yesterday...
And as I know not being fully fit on the code I really
try to run the tests before doing the commit - and they passed for
shared as wellas for apacheds. Is there a must using SunJDK instead
JRokit or did I got the wrong command for running all the tests?
No. tests should pass with both of them. In any case, the pb was totally
JVM independant.
Or is
the test of apacheds also mandatory when doing any changes in shared?
Yes, and that means we don't test enough in shared :/
It
took for apacheds about 7 minutes 20 seconds - is this long enough?
IMHO, it's way to long, but it's hard to make it shorter ... That's a
real bummer to have to wait more than 10 minutes before committing, if
you want my opinion, but this is the only way to protect ourselves.
BTW
Some of the equals methods are still breaking the contract of symmetric,
transitive [1] because of using reference comparison for collections
[2], e.g. [3]. Is this wanted (there are more such cases).
[1]
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Object.html#equals%28java.lang.Object%29
[2]
http://java.sun.com/j2se/1.5.0/docs/api/java/util/Collection.html#equals%28java.lang.Object%29
[3]
http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/aci/ProtectedItem.html#267
I don't know about protectedItem, I had no time yesterday to review this
piece of code. I must also admit this is not a part of the code I know...
FYI, I had to remove most of the ObjectClass.hashcode() containt to make
maven happy. I'm not pleased with the code I put instead, as it's a
gross approximation of what the hashcode should be, but anyway, it
should work.
There is one point whihi is important too when implementing hashcode()
methods : performances. In many cases, we don't want to compute those
hashcode everytime, this we store the result in a static variable. This
is cumbersome because it leads to either static initialization (not
always possible) or to some level of synchronization (sucks ...)
However, we have to keep this in mind, as it's really impact the server.
That being said, good job !
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
