You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/10/14 14:07:40 UTC

Re: svn commit: r27172 - in trunk/subversion: libsvn_ra_neon libsvn_ra_serf libsvn_ra_svn mod_dav_svn/reports

vgeorgescu@tigris.org writes:
> Log:
> Only use the depth-filtering editor if the server doesn't understand depths.
>
> For ra_neon/ra_serf, we already do this (see r25990), by looking for a 
> "depth" attribute in the response sent by mod_dav_svn.  Since r27168, we 
> can use capabilities instead, so let's do that.

Ooh, good idea!  I didn't even think of that.

> --- trunk/subversion/libsvn_ra_neon/fetch.c	(original)
> +++ trunk/subversion/libsvn_ra_neon/fetch.c	Sat Oct 13 04:32:03 2007
> @@ -3079,6 +3050,29 @@
>    report_baton_t *rb;
>    const char *s;
>    svn_stringbuf_t *xml_s;
> +  const svn_delta_editor_t *filter_editor;
> +  void *filter_baton;
> +  svn_boolean_t has_target = *target ? TRUE : FALSE, new_server;

Better to declare 'new_server' on its own line, it's easy to lose
track of it in the conditional definition preceding it.

Also, instead of 'new_server', how about 'server_supports_depth' or
something?  These servers won't always be "new"... someday, they'll
even be "old" :-).

> @@ -3098,7 +3092,6 @@
>    rb->svndiff_decoder = NULL;
>    rb->base64_decoder = NULL;
>    rb->cdata_accum = svn_stringbuf_create("", pool);
> -  rb->depth = (depth == svn_depth_unknown ? svn_depth_infinity : depth);
>    rb->send_copyfrom_args = send_copyfrom_args;
>  
>    /* Neon "pulls" request body content from the caller. The reporter is

I think the log message doesn't mention this change.  (Though I'm very
glad to see it go -- that line always bothered me, philosophically :-) ).

[Similar comments apply to the ra_serf changes.]

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r27172 - in trunk/subversion: libsvn_ra_neon libsvn_ra_serf libsvn_ra_svn mod_dav_svn/reports

Posted by Vlad Georgescu <vg...@gmail.com>.
Karl Fogel wrote:
>> --- trunk/subversion/libsvn_ra_neon/fetch.c	(original)
>> +++ trunk/subversion/libsvn_ra_neon/fetch.c	Sat Oct 13 04:32:03 2007
>> @@ -3079,6 +3050,29 @@
>>    report_baton_t *rb;
>>    const char *s;
>>    svn_stringbuf_t *xml_s;
>> +  const svn_delta_editor_t *filter_editor;
>> +  void *filter_baton;
>> +  svn_boolean_t has_target = *target ? TRUE : FALSE, new_server;
> 
> Better to declare 'new_server' on its own line, it's easy to lose
> track of it in the conditional definition preceding it.
> 
> Also, instead of 'new_server', how about 'server_supports_depth' or
> something?  These servers won't always be "new"... someday, they'll
> even be "old" :-).

Thanks for the review.  I made these changes in r27193.

-- 
Vlad

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org