Re: svn_ra_get_file_revs2 vs. blame vs. FS API

[Stefan Fuhrmann <st...@wandisco.com>]

On Sun, Feb 23, 2014 at 11:33 PM, Stefan Fuhrmann <
stefan.fuhrmann@wandisco.com> wrote:

> On Fri, Feb 21, 2014 at 6:01 PM, Julian Foad <ju...@btopenworld.com>wrote:
>
>> Stefan Fuhrmann wrote:
>> > Julian Foad wrote:
>> >> Stefan Fuhrmann wrote:
>>>
>>> >> or something very much like that. Those assumptions look like the
>>> real problem here.
>>> >>
>>> >>
>>> >>> I see the following options:
>>> >>>
>>> >>> * Redefine svn_fs_contents_changed and svn_fs_props_changed to match
>>> >>>   the current implementation. I'd rather not but that just a -0 from
>>> my side.
>>> >>
>>> >> We should deprecate them, and adjust their doc strings with a
>>> historical note
>>> >> about the discrepancy, and replace them with one or two new (pairs
>>> of) APIs:
>>> >>
>>> >>  -- a quick optimization API -- the definitely/maybe question -- like
>>> the
>>> >>  existing implementations but documented properly and named
>>> appropriately; and
>>> >>
>>> >>  -- (perhaps) a definite content comparison API: "has the content
>>> changed?"
>>> >
>>> > I tend to prefer an extra flag parameter in a bumped version of the two
>>> > API functions as it makes existing API users aware of how the API has
>>> > been changed.
>>>
>>> Well, this is just a general API design style issue, but I think a name
>>> change is equally visible. I think a boolean parameter is appropriate where
>>> the caller is likely to choose from both variants at run time, but in this
>>> case I think callers will choose one or the other at design time, never at
>>> run time. I think we have far too many inappropriate boolean behaviour
>>> modifiers already in Subversion APIs. I think they make the call sites hard
>>> to read.
>>>
>>
>> Fair point. I was thinking about revving svn_ra_get_file_revs2
> as well with the same new option (callers can chose whether
> need exact reports or want to process the delta anyway).
> That would be a pass-through value for svn_fs_*_changed.
>

I implemented just that in my working copy and it turned out
to be a mess. Every RA layer had to be updated and in the
end, there would only be the blame function to use it - with
some fixed value for the STRICT option.

So, I decided to go with the separate API functions for the
different behaviors in r1573111.

-- Stefan^2.

Re: svn_ra_get_file_revs2 vs. blame vs. FS API

[Julian Foad <ju...@btopenworld.com>]

Stefan Fuhrmann wrote:
> Stefan Fuhrmann wrote:
>> Julian Foad wrote:
>>> Stefan Fuhrmann wrote:
>>>> Julian Foad wrote:
>>>>>  -- a quick optimization API -- the definitely/maybe question -- like the
>>>>>  existing implementations but documented properly and named appropriately; and
>>>>>
>>>>>  -- (perhaps) a definite content comparison API: "has the content changed?"
>>>>
>>>> I tend to prefer an extra flag parameter in a bumped version [...]
>>> 
>>> Well, this is just a general API design style issue, but I think [...]
>> 
>> Fair point. I was thinking about revving svn_ra_get_file_revs2
>> as well with the same new option (callers can chose whether
>> need exact reports or want to process the delta anyway).
>> That would be a pass-through value for svn_fs_*_changed.
>
> I implemented just that in my working copy and it turned out
> to be a mess. Every RA layer had to be updated and in the
> end, there would only be the blame function to use it - with
> some fixed value for the STRICT option.
>
> So, I decided to go with the separate API functions for the
> different behaviors in r1573111.

Looks good.

Thanks.
- Julian