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
committed.

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

Regards,
Graham
-- 
-----------------------------------------
[EMAIL PROTECTED]               "There's a moon
                                        over Bourbon Street
                                                tonight..."

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to