Re: [PATCH] Infrastructure changes for incomplete directory support.

On 8/8/06, Karl Fogel <kfogel@google.com> wrote:

> +/** The WebDAV concept of "depth".
> + * @since New in 1.5.
> + */

Any reason to specifically refer to it as 'The WebDAV concept", it's
not like it's limited to use in the DAV layer...

> +typedef enum
> +{
> + svn_depth_exclude = -1, /* Exclude (remove, whatever) directory D. */

I don't notice any use of this one in the rest of the patch...

> + svn_depth_zero = 0, /* Just the named directory D, no entries. */
> + svn_depth_one = 1, /* D + immediate children (D and its entries) */
> + svn_depth_infinity = 2, /* D + all descendants (full recursion from D) */
> +} svn_depth_t;

> + /* Depth. */
> + {
> + const char *result;
> + SVN_ERR(read_val(&result, buf, end));
> + if (result[0] == '0' && result[1] == '\0')
> + entry->depth = svn_depth_zero;
> + if (result[0] == '1' && result[1] == '\0')
> + entry->depth = svn_depth_one;
> + else
> + /* svn_depth_exclude "can't happen" here, so assume infinity. */
> + entry->depth = svn_depth_infinity;
> + }
> +

It occurs to me here while reading the serialization/deserialization
code that maybe it would be nice to have names other than "zero" or
"one", since unless you already know what those mean they don't really
describe much to you. They also look like any other line in the
entries file, which is already somewhat hard to read. Might be nice
if they stood out more...

I don't actually have any ideas, mind you, but in the "it would be
nice" category...

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 9 18:50:19 2006