Re: svn commit: r955136 - in /subversion/trunk/subversion: include/ libsvn_fs/ libsvn_fs_base/ libsvn_fs_fs/ tests/libsvn_fs/

On Wed, 2010-06-16 at 14:30 +0100, Philip Martin wrote:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>
> > Philip Martin wrote on Wed, 16 Jun 2010 at 11:17 -0000:
> >> How are you going to check adds? During an add the old value is NULL
> >> but must still be checked.
> >>
> >
> > During an add, old_value_p!=NULL and *old_value_p==NULL, and the code
> > checks that present_value==NULL as well.
> >
> > The case old_value_p==NULL means "unspecified" (i.e., just make the
> > change regardless of what's there now).
> >
>
> OK, I didn't read the code carefully enough.

I didn't spot that either. Please could you add a second "const" in the
function prototype to make it clearer? Otherwise it looks like an
output.

"const svn_string_t *const *old_p"

- Julian

> >> I don't think the old interface should be deprecated.
> >>
> >
> > Why? The new interface has all the functionality of the old interface;
> > one can do
> >
> > s/svn_fs_change_rev_prop(*args)/svn_fs_change_rev_prop2(args, old_value_p=NULL)/g
> >
> > with no change in functionality.
>
> Agreed.
>
Received on 2010-06-16 16:06:37 CEST