directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Karasulu" <akaras...@apache.org>
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 17:02:32 GMT
On Thu, Jul 31, 2008 at 3:58 PM, <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
> 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?


>
> +    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

Mime
View raw message