On Mon, Nov 30, 2009 at 09:30:58PM +0530, Kannan wrote:
> Stefan Sperling wrote:
> >> @@ -773,7 +777,7 @@
> >>  
> >>           Note that we're not sending the locks in the If: header, for
> >>           the same reason we're not sending in MERGE's headers: httpd has
> >> -       limits on the amount of data it's willing to receive in headers. */
> >> +         limits on the amount of data it's willing to receive in headers. 
> >> */
> > 
> > Why was this changed?
> 
> Indentation fix in the comment.

It's better to make unrelated changes in separate patches.

I found one more nit:

> Index: subversion/libsvn_ra_neon/props.c
> ===================================================================
> --- subversion/libsvn_ra_neon/props.c (revision 885339)
> +++ subversion/libsvn_ra_neon/props.c (working copy)
> @@ -991,7 +991,10 @@
>  
>    /* maybe return bc_url to the caller */
>    if (bc_url)
> -    *bc_url = *my_bc_url;
> +    {
> +      bc_url->data = svn_uri_canonicalize(my_bc_url->data, pool);
> +      bc_url->len = my_bc_url->len;
> +    }

It would be nicer to have svn_ra_neon__get_baseline_props()
do the canonicalisation, at this spot:

  /* don't forget to tack on the parts we lopped off in order to find
     the VCC...  We are expected to return a URI decoded relative
     path, so decode the lopped path first. */
  my_bc_relative = svn_path_join(relative_path->data,
                                 svn_path_uri_decode(lopped_path, pool),
                                 pool);

Then the caller would not need to worry about canonicalisation
and your above change would not be needed.

Also we could replace the svn_path_join() while there.

The rest looks good now.

Thanks,
Stefan

Reply via email to