apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: Fwd: Re: [PATCH] LDAP support for apr-util
Date Tue, 31 Jul 2001 06:40:24 GMT
Ryan Bloom wrote:

> 1)  This is FAR too large to review.  It would be much easier if we started
>  with something small, and allowed the number of functions to grow as we need
>  them.

This is a port of existing code, which will break the compile unless you
include it as a unit.

At this stage I am most interested in "does this thing break the
compile" - obviously it's too big to swallow in a single step. 

> 2)  There are C++ comments littered throughout the code.  We can't use C++
>  comments, because many compilers throw up on them.

C++ comments here means "stuff that's currently broken and must be fixed
before commiting". They will be gone when the code is ready to be

> 3)  The apr_ldap.h header file has a bunch of functions that aren't
>  documented at all.  No function should ever be committed to APR or APR-util
>  without docs.
> 4)  The docs for the ldap structures won't generate doxygen docs.  Again, no
>  structure should ever be committed that way.

Is there a guide somewhere on how to add these docs?

> That's it for now.  This is not an exhaustive review, because the patch is
>  too large to spend that much time on.

This is all I needed for now - I know it's big, which is why I wanted to
focus on build and header file issues first.

Do you have any comments on the patch to configure.in and apu-conf.m4? I
am keen to know whether what's being done there is the right approach.

minfrin@sharp.fm		"There's a moon
					over Bourbon Street
View raw message