directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel L├ęcharny <elecha...@apache.org>
Subject Re: Trunk is building again : Next steps
Date Tue, 01 Feb 2011 16:25:06 GMT
On 2/1/11 4:56 PM, Alex Karasulu wrote:
> On Tue, Feb 1, 2011 at 5:37 PM, Emmanuel Lecharny<elecharny@gmail.com>wrote:
>
>> Hi guys,
>>
>> So we get a building trunk as of today.
>>
>> We can now move to the next step. There is some cleanup to do in the code
>> we injected last week, as we were hurrying to get something that works. Here
>> is a list of tasks to complete :
>> - Review the CodecService usage
>>
> +1
>
>
>> - Remove the CodecService from the DSML grammars
>>
> +1 and some other areas:
>
>     Where we create new DefaultLdapCodecService instances?
>
>
> shared-ldap-client-api     =>  LdapNetworkConnection<initialization>
>
>     - needed
>
> apacheds-core                  =>  DefaultDirectoryService.initialize()
>
>     - needed
>
> apacheds-core-api           =>  LdapCoreSessionConnection()<initialization>
>
>     - totally unnecessary: the DirectoryService can be used to get a handle
>
> shared-dsmlv2-parser     =>  Dsmlv2Grammar<initialization>
>
>     - unnecessary
>
> shared-dsmlv2-parser     =>  Dsmlv2ResponseGrammar<initialization>
>
> shared-ldap                       =>  LdapEncoder<initialization>
>
>     - review

The encoder should not create a CodecService. It's never called out of 
the blue, so the caller can pass it to the encoder.
> studio-connection.core    =>  CursorStudioNamingEnumeration<initialization>
>
>     - unnecessary
>
> studio-ldapbrowser.core =>  ExportDsmlRunnable<initialization>
>
>     - unnecessary
>
> studio-ldapbrowser.core =>  ImportDsmlRunnable<initialization>
>
>     - unnecessary
>
> Can review again after dust settles to get this organized correctly.

>
>> - Rename the *I*xxx interfaces
>>
> I don't like this Ixxx interface naming but it did help in many places to my
> surprise. It for example narrows searching for an interface to implement a
> great deal because of the I in front. I think it should be considered - at
> least we need definitive consensus on whether to go with it or not.
>
> I can go either way - I just don't think this should be turned into a "oh no
> not again this converstation." Our situation in releasing an API warrants it
> one last time at most.
In this case, none of those *I*xxx interfaces are visible from the API.

As we have some inconsistent code atm, I'd suggest we stick to what we 
decided years ago and get back to the standard notation (ie Xxx for 
interfaces and XxxImpl for implementations).
>> - Review the contol encoding (we need to avoid a double call to the
>> computeLength() method)
>>
> Yep and we need to make sure we're not double wrapping with decorators.
I had trouble with this piece of code.

Right now, to get it working, I moved the control envelop encoding out 
of the ControlDecorator class. The reason was that the encode() methods 
was used to encode the Control's value.

The problem with this approach is that we can't anymore stored the 
control's length when we have computed it in the control, so when we 
encode it, we have to compute it again.

I'd like to add a computeEnvelopLength() and encodeEnvelop() methods in 
the ControlDecorator, and move back the code from the LdapEncoder class 
down to this class.

wdyt ?

>> - Add the missing Javadocs
>>
> +1
>
>
>> - Relome the duplicated fields
>>
> +1

I think it's already done
>
>> Once done, we will have some refactoring to do :
>> - move the controls in one single package (right now, they are spread in
>> any places)
>>
> +1
>
> I can push a separate thread on this matter since it might distract from the
> high level discussion we have here.
yes, a separate thread could be good, but I don't think there is much to 
discuss here. It's probably more about giving some information.
>> - check that we have a clear separation between teh API and the SPI (the
>> extended operation might be a problem here)
>>
> +1
>
> What's been done for controls need to be done for extended operations.

Yep.
>> Last, not least, the PasswordPolicy tests have been ignored, we shoumd move
>> them to server-integ and make them work.
>>
>>
> +1
>
> We need to get all server tests out of core and core-api. There are many of
> them.
hmmm, not sure.

We need a list here.

Let's start !

PS : And remember : no trunk breakage, no big refactoring without 
previous discussion on the ML, branching could be the way to go.

-- 
Regards,
Cordialement,
Emmanuel L├ęcharny
www.iktek.com


Mime
View raw message