directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kiran Ayyagari <ayyagariki...@gmail.com>
Subject Re: svn commit: r681463 - in /directory/apacheds/branches/bigbang/core/src/main/java/org/apache/directory/server/core/changelog: ChangeLog.java DefaultChangeLog.java
Date Fri, 01 Aug 2008 19:13:39 GMT


Alex Karasulu wrote:
>
> On Thu, Jul 31, 2008 at 3:58 PM, <kayyagari@apache.org 
> <mailto:kayyagari@apache.org>> wrote:
>
>     Author: kayyagari
>     Date: Thu Jul 31 12:58:02 2008
>     New Revision: 681463
>
>     URL: http://svn.apache.org/viewvc?rev=681463&view=rev
>     <http://svn.apache.org/viewvc?rev=681463&view=rev>
>     Log:
>     made the changelog partition suffix configurable
>     included the default values for the same
>
>     +    void setPartitionSuffix( String suffix );
>     +
>
>
> This is a DN I take it?
>  
yep
>
>
>     +    void setRevOuSuffix( String revOuSuffix );
>     +
>
>
> This is not a DN?  Is it the value of the ou attribute?  There is no 
> javadocs here.  Please do not add interface methods without javadocs. 
>  
>
>
>     +    void setTagOuSuffix( String tagOuSuffix );
>
>
>
> Without Javadocs to help understand I think these names are not very 
> intuitive especially since suffix is usually used to mean the base DN 
> of a partition attached to the DIT: the namingContext.  Perhaps 
> setRevisionsContainerValue() and setTagContainerValue() or 
> setRevisionsContainerName() and setTagContainerName() might be best.  
> Don't know but it's better than using suffix. 
>
> Providing a DN may be an issue because then your container has to 
> allow the presence of the rdn value.  Might give too much freedom 
> where you want to constrain.
>
> Please add some documentation so these new interface methods better 
> understood: we cannot accept undocumented code in general but with key 
> interfaces that are part of an SPI in ApacheDS it's even more 
> unacceptible.  Please don't make a habit of this.
>
> Alex
>
>

my bad missed the javadoc for these, thanks for letting me know, have 
fixed it now.


Kiran Ayyagari

Mime
View raw message