Re: PATCH: WIN32 libsvn_ra_dav

Hi, a few requests I can safely make on behalf of the team:

First, please read the HACKING file in the top level, particularly the
part about writing log messages.

Second, "//" only works as a commenting mechanism in C++ (although
it's an extension in some C compilers); Subversion is written in C.

Third, it's important that we do things right, and not merely solve
the problem of the moment. For instance:

-#include <strings.h> /* for strcasecmp() (### should come from APR) */
+// #include <strings.h> /* for strcasecmp() (### should come from APR)

The previous code was definitely wrong (nobody should be using
<strings.h> in new code), but simply commenting it out is likely to
break the build on other platforms. The right answer is to include
<string.h> instead. (As Greg Stein is likely to point out if I don't,
one could also define the appropriate symbol and include <apr_want.h>,
but we have traditionally just included <string.h> because we assume
ANSI C).

I understand that sometimes the right answer is difficult, or requires
specialized knowledge; in that case, the right answer is to point out
the problem and not submit a patch.

Following these guidelines will help people review your patches more
easily.
Received on Sat Oct 21 14:36:28 2006