Daniel Rall wrote:
> Hi Mike! A few questions/comments...
>
> On Fri, 10 Aug 2007, cmpilato@tigris.org wrote:
> ...
>> Fix a regression I caused in ra_svn with the sparse directories compat
>> code.
>>
>> * subversion/include/svn_delta.h,
>> * subversion/libsvn_delta/depth_filter_editor.c
>> (svn_delta_depth_filter_editor): Allow svn_depth_unknown (as a no-op).
>>
>> * subversion/libsvn_ra_svn/client.c
>> (DEPTH_FILTER_EDITOR): New.
>
> I don't see such a symbol in this commit. Looks like you meant to
> refer to the new DEPTH_TO_RECURSE() macro instead.
>
>> (normalize_depth_values): Remove as unused.
>> (ra_svn_update, ra_svn_switch, ra_svn_diff, ra_svn_status): Stop using
>> normalize_depth_values(), stop clobbering depth, and setup the
>> recurse value ourselves now.
>
> How about "Replace usage of normalize_depth_values() with
> DEPTH_TO_RECURSE(), ..."?
Fixed.
>> --- trunk/subversion/libsvn_delta/depth_filter_editor.c (original)
>> +++ trunk/subversion/libsvn_delta/depth_filter_editor.c Fri Aug 10 13:21:00 2007
>> @@ -397,8 +397,11 @@
>> struct edit_baton *eb;
>>
>> /* Easy out: if the caller wants infinite depth, there's nothing to
>> - filter, so just return the editor we were supposed to wrap. */
>> - if (requested_depth == svn_depth_infinity)
>> + filter, so just return the editor we were supposed to wrap. And
>> + if they've asked for an unknown depth, we can't possibly know
>> + what that means, so why bother? */
>> + if ((requested_depth == svn_depth_unknown)
>> + || (requested_depth == svn_depth_infinity))
>
> This conditional is the functional equivalent of the
> SVN_DEPTH_TO_RECURSE() macro (from svn_types.h). Would it make sense
> to use the macro instead?
No. The conditionals might match, but the semantics don't. This isn't a
check for "is-recursive" like SVN_DEPTH_TO_RECURSE() would offer.
>> +#define DEPTH_TO_RECURSE(d) \
>> + (((d) == svn_depth_unknown || (d) > svn_depth_files) ? TRUE : FALSE)
> ...
>
> This macro definition differs from SVN_DEPTH_TO_RECURSE(). Might be
> worth mentioning why here in an inline comment.
Will do.
--
C. Michael Pilato <cmpilato@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Wed Aug 15 20:04:52 2007