On Sun, Feb 23, 2014 at 11:33 PM, Stefan Fuhrmann <
stefan.fuhrmann_at_wandisco.com> wrote:
> On Fri, Feb 21, 2014 at 6:01 PM, Julian Foad <julianfoad_at_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.
Received on 2014-03-01 00:27:22 CET